From d1b28af94144c7b4ceb92a017418163e4499d72d Mon Sep 17 00:00:00 2001 From: Your Name <5115845+dauglyon@users.noreply.github.com> Date: Fri, 29 May 2026 15:32:06 -0700 Subject: [PATCH 1/3] Validate signup username against backend format rules The frontend availability check compared availablename to username.toLowerCase(), so a username like "John" passed validation but was then rejected by auth2 for the uppercase letter. Inputs with characters auth2 strips (dots, hyphens, etc.) failed with a misleading "Username is not available" message. Mirror the kbase/auth2 NewUserName rules on the form: must start with a lowercase letter, only [a-z0-9_], no repeating or trailing underscores, at most 100 chars. Show a specific error for format violations and only treat availability mismatches as collisions. --- .../signup/AccountInformation.test.tsx | 68 +++++++++++++++++++ src/features/signup/AccountInformation.tsx | 41 +++++++++-- 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/src/features/signup/AccountInformation.test.tsx b/src/features/signup/AccountInformation.test.tsx index d491669b..82911794 100644 --- a/src/features/signup/AccountInformation.test.tsx +++ b/src/features/signup/AccountInformation.test.tsx @@ -135,4 +135,72 @@ describe('AccountInformation', () => { expect(mockNavigate).toHaveBeenCalledWith('/signup/3'); }); + + test.each([ + ['uppercase letters', 'BadUser'], + ['a leading digit', '1baduser'], + ['a hyphen', 'bad-user'], + ['a period', 'bad.user'], + ['repeating underscores', 'bad__user'], + ['a trailing underscore', 'baduser_'], + ])( + 'blocks submission and shows format error for username with %s', + async (_label, badName) => { + const store = createTestStore(); + store.dispatch( + setLoginData({ + creationallowed: true, + expires: 0, + login: [], + provider: 'Google', + create: [ + { + provemail: 'test@test.com', + provfullname: 'Test User', + availablename: 'testuser', + id: '123', + provusername: 'testuser', + }, + ], + }) + ); + renderWithProviders(, { store }); + + await act(() => { + fireEvent.change(screen.getByRole('textbox', { name: /Full Name/i }), { + target: { value: 'Test User' }, + }); + }); + await act(() => { + fireEvent.change(screen.getByRole('textbox', { name: /Email/i }), { + target: { value: 'test@test.com' }, + }); + }); + await act(() => { + fireEvent.change( + screen.getByRole('textbox', { name: /KBase Username/i }), + { target: { value: badName } } + ); + }); + await act(() => { + fireEvent.change( + screen.getByRole('textbox', { name: /Organization/i }), + { target: { value: 'Test Org' } } + ); + }); + await act(() => { + fireEvent.change(screen.getByRole('textbox', { name: /Department/i }), { + target: { value: 'Test Dept' }, + }); + }); + await act(() => { + fireEvent.submit(screen.getByTestId('accountinfoform')); + }); + + expect( + screen.getByText(/may contain only lowercase letters/i) + ).toBeInTheDocument(); + expect(mockNavigate).not.toHaveBeenCalledWith('/signup/3'); + } + ); }); diff --git a/src/features/signup/AccountInformation.tsx b/src/features/signup/AccountInformation.tsx index 156bd49a..6aeeefaf 100644 --- a/src/features/signup/AccountInformation.tsx +++ b/src/features/signup/AccountInformation.tsx @@ -58,8 +58,14 @@ export const AccountInformation: FC<{}> = () => { const [username, setUsername] = useState(account.username ?? ''); const userAvail = loginUsernameSuggest.useQuery(username); const nameShort = username.length < 3; - const nameAvail = - userAvail.currentData?.availablename === username.toLowerCase(); + const nameTooLong = username.length > 100; + // Mirrors backend rules in kbase/auth2 NewUserName: must start with a + // lowercase letter; only [a-z0-9_]; no repeating or trailing underscores. + const nameFormatValid = + /^[a-z][a-z0-9_]*$/.test(username) && + !username.includes('__') && + !username.endsWith('_'); + const nameAvail = userAvail.currentData?.availablename === username; const surveyQuestion = 'How did you hear about us? (select all that apply)'; const [optionalText, setOptionalText] = useState>({}); @@ -199,7 +205,11 @@ export const AccountInformation: FC<{}> = () => { required: true, onChange: (e) => setUsername(e.currentTarget.value), validate: () => - !nameShort && !userAvail.isFetching && nameAvail, + !nameShort && + !nameTooLong && + nameFormatValid && + !userAvail.isFetching && + nameAvail, })} defaultValue={account.username} helperText={ @@ -209,6 +219,24 @@ export const AccountInformation: FC<{}> = () => { Username is too short.
+ ) : nameTooLong ? ( + + Username must be at most 100 characters. +
+ + ) : !nameFormatValid ? ( + + Username may contain only lowercase letters, digits, and + underscores, and must start with a letter. Underscores + cannot repeat or end the username. + {userAvail.currentData?.availablename ? ( + <> + {' '} + Suggested: "{userAvail.currentData.availablename}". + + ) : null} +
+ ) : !nameAvail && !userAvail.isFetching ? ( Username is not available. Suggested: " @@ -224,7 +252,12 @@ export const AccountInformation: FC<{}> = () => { } - error={nameShort || (!userAvail.isFetching && !nameAvail)} + error={ + nameShort || + nameTooLong || + !nameFormatValid || + (!userAvail.isFetching && !nameAvail) + } /> From ec1804fd53fb3d7e99033dfd1bf1337811c28ece Mon Sep 17 00:00:00 2001 From: Your Name <5115845+dauglyon@users.noreply.github.com> Date: Fri, 29 May 2026 15:32:20 -0700 Subject: [PATCH 2/3] Gate CDM sidebar link on BERDL_USER role The CDM nav item was gated on CDM_JUPYTERHUB_ADMIN, so only admins saw the link. Per the BERDL platform docs, BERDL_USER is the role that gates access to the lakehouse; CDM_JUPYTERHUB_ADMIN is a separate admin role for approving access requests. --- src/features/layout/LeftNavBar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/features/layout/LeftNavBar.tsx b/src/features/layout/LeftNavBar.tsx index 32817209..e6274be1 100644 --- a/src/features/layout/LeftNavBar.tsx +++ b/src/features/layout/LeftNavBar.tsx @@ -72,7 +72,7 @@ const LeftNavBar: FC = () => { icon={faDatabase} badge={'alpha'} badgeColor={'warning'} - requiredRole="CDM_JUPYTERHUB_ADMIN" + requiredRole="BERDL_USER" />
    From 349922d74a8328b117e28a9b87cb2dbeb4882243 Mon Sep 17 00:00:00 2001 From: Your Name <5115845+dauglyon@users.noreply.github.com> Date: Mon, 1 Jun 2026 09:17:07 -0700 Subject: [PATCH 3/3] Skip user username pre-fill on ORCID signup ORCID's provider-supplied username is the numeric ORCID iD (e.g. 0000-0002-1825-0097). auth2's NewUserName.sanitizeName strips all of that to empty and getAvailableUserName falls back to user, so every ORCID signup landed on the form with user1 (or user2, ...) already in the username field. Leave the username blank when the provider is OrcID so the user picks their own. Display name and email pre-fill are unchanged. --- src/features/signup/SignupSlice.test.tsx | 43 ++++++++++++++++++++++++ src/features/signup/SignupSlice.tsx | 13 +++++-- 2 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 src/features/signup/SignupSlice.test.tsx diff --git a/src/features/signup/SignupSlice.test.tsx b/src/features/signup/SignupSlice.test.tsx new file mode 100644 index 00000000..9762bf9d --- /dev/null +++ b/src/features/signup/SignupSlice.test.tsx @@ -0,0 +1,43 @@ +import { createTestStore } from '../../app/store'; +import { setLoginData } from './SignupSlice'; + +const makeLoginData = ( + provider: string, + availablename: string +): Parameters[0] => ({ + creationallowed: true, + expires: 0, + login: [], + provider, + create: [ + { + provemail: 'jane@example.com', + provfullname: 'Jane Doe', + availablename, + id: '123', + provusername: '0000-0002-1825-0097', + }, + ], +}); + +describe('signup setLoginData', () => { + test('pre-fills username from availablename for non-ORCID providers', () => { + const store = createTestStore(); + store.dispatch(setLoginData(makeLoginData('Google', 'janedoe'))); + expect(store.getState().signup.account.username).toBe('janedoe'); + }); + + test('leaves username blank for ORCID logins to avoid user default', () => { + const store = createTestStore(); + store.dispatch(setLoginData(makeLoginData('OrcID', 'user1'))); + expect(store.getState().signup.account.username).toBeUndefined(); + }); + + test('still pre-fills display name and email for ORCID', () => { + const store = createTestStore(); + store.dispatch(setLoginData(makeLoginData('OrcID', 'user1'))); + const account = store.getState().signup.account; + expect(account.display).toBe('Jane Doe'); + expect(account.email).toBe('jane@example.com'); + }); +}); diff --git a/src/features/signup/SignupSlice.tsx b/src/features/signup/SignupSlice.tsx index 7c398b45..ec5d6fbd 100644 --- a/src/features/signup/SignupSlice.tsx +++ b/src/features/signup/SignupSlice.tsx @@ -44,9 +44,16 @@ export const signupSlice = createSlice({ // Set provider creeation data state.loginData = action.payload; // Set account defaults from provider - state.account.display = action.payload?.create[0].provfullname; - state.account.email = action.payload?.create[0].provemail; - state.account.username = action.payload?.create[0].availablename; + const detail = action.payload?.create[0]; + state.account.display = detail?.provfullname; + state.account.email = detail?.provemail; + // ORCID's provusername is the numeric ORCID iD, which auth2 cannot + // sanitize into a valid username and falls back to user. Leave the + // field blank for ORCID so the user picks their own. + state.account.username = + action.payload?.provider === 'OrcID' + ? undefined + : detail?.availablename; }, setAccount: ( state,