diff --git a/.github/workflows/reusable_build-push.yml b/.github/workflows/reusable_build-push.yml index d68782cd..3ba43911 100644 --- a/.github/workflows/reusable_build-push.yml +++ b/.github/workflows/reusable_build-push.yml @@ -14,6 +14,9 @@ on: jobs: build-push-image: runs-on: ubuntu-latest + permissions: + contents: read + packages: write steps: - name: Check out GitHub Repo uses: actions/checkout@v3 @@ -26,12 +29,11 @@ jobs: uses: docker/setup-buildx-action@v2 - name: Login to GHCR - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ghcr.io - # note that the calling workflow must set `secrets: inherit` - username: '${{ secrets.GHCR_USERNAME }}' - password: '${{ secrets.GHCR_TOKEN }}' + username: ${{ github.actor }} + password: ${{ github.token }} - name: Set up Node.js uses: actions/setup-node@v2 diff --git a/config.json b/config.json index 70b54adb..340d8365 100644 --- a/config.json +++ b/config.json @@ -10,7 +10,7 @@ "domain": "ci.kbase.us", "legacy": "legacy.ci.kbase.us", "public_url": "/", - "cdm_domain": "cdmhub.ci.kbase.us" + "lakehouse_domain": "cdmhub.ci.kbase.us" }, "ci-europa": { @@ -47,7 +47,7 @@ "name": "kbase_session_backup", "domain": ".kbase.us" }, - "cdm_domain": "hub.berdl.kbase.us", + "lakehouse_domain": "hub.berdl.kbase.us", "redirect_whitelist": ["*.berdl.kbase.us"] } } diff --git a/scripts/build_deploy.ts b/scripts/build_deploy.ts index 623e17ae..b7c56dca 100755 --- a/scripts/build_deploy.ts +++ b/scripts/build_deploy.ts @@ -18,7 +18,7 @@ interface EnvironmentConfig { name: string; domain: string; }; - cdm_domain?: string; + lakehouse_domain?: string; redirect_whitelist?: string[]; } @@ -54,7 +54,7 @@ const setEnvironment = ( legacy, public_url: publicURL, backup_cookie: backupCookie, - cdm_domain: cdmDomain, + lakehouse_domain: lakehouseDomain, redirect_whitelist: redirectWhitelist, } = environmentConfig; @@ -66,7 +66,7 @@ const setEnvironment = ( REACT_APP_KBASE_LEGACY_DOMAIN: legacy, REACT_APP_KBASE_BACKUP_COOKIE_NAME: backupCookie?.name || '', REACT_APP_KBASE_BACKUP_COOKIE_DOMAIN: backupCookie?.domain || '', - REACT_APP_KBASE_CDM_DOMAIN: cdmDomain || 'cdmhub.' + domain, + REACT_APP_KBASE_LAKEHOUSE_DOMAIN: lakehouseDomain || 'cdmhub.' + domain, REACT_APP_REDIRECT_WHITELIST: redirectWhitelist?.join(',') || '', }; Object.assign(process.env, envsNew); diff --git a/src/app/Routes.tsx b/src/app/Routes.tsx index ccbfd694..40fb77d3 100644 --- a/src/app/Routes.tsx +++ b/src/app/Routes.tsx @@ -38,7 +38,7 @@ import { LogInSessions } from '../features/account/LogInSessions'; import { UseAgreements } from '../features/account/UseAgreements'; import { skipToken } from '@reduxjs/toolkit/dist/query'; import { getMe } from '../common/api/authService'; -import { CDMRedirect } from '../features/cdm/CDMRedirect'; +import { LakehouseRedirect } from '../features/lakehouse/LakehouseRedirect'; import { OrcidLink, OrcidLinkContinue, @@ -147,10 +147,19 @@ const Routes: FC = () => { } /> - {/* CDM */} - - } />} /> + {/* Lakehouse */} + + } />} + /> + {/* The CI hub's login page (kbase/cdm-jupyterhub templates/login.html) + still sends users to the pre-rename route. */} + } + /> {/* IFrame Fallback Routes */} diff --git a/src/features/cdm/CDMRedirect.tsx b/src/features/lakehouse/LakehouseRedirect.tsx similarity index 81% rename from src/features/cdm/CDMRedirect.tsx rename to src/features/lakehouse/LakehouseRedirect.tsx index 656c47e7..027670b4 100644 --- a/src/features/cdm/CDMRedirect.tsx +++ b/src/features/lakehouse/LakehouseRedirect.tsx @@ -2,9 +2,9 @@ import { Container, Stack } from '@mui/system'; import { useEffect } from 'react'; import { Loader } from '../../common/components'; -export const CDMRedirect = () => { +export const LakehouseRedirect = () => { useEffect(() => { - window.location.href = `https://${process.env.REACT_APP_KBASE_CDM_DOMAIN}/hub`; + window.location.href = `https://${process.env.REACT_APP_KBASE_LAKEHOUSE_DOMAIN}/hub`; }); return ( @@ -16,7 +16,7 @@ export const CDMRedirect = () => { justifyContent={'center'} > -
Redirecting to CDM
+
Redirecting to Lakehouse
); diff --git a/src/features/layout/LeftNavBar.tsx b/src/features/layout/LeftNavBar.tsx index 32817209..f50c0923 100644 --- a/src/features/layout/LeftNavBar.tsx +++ b/src/features/layout/LeftNavBar.tsx @@ -67,12 +67,12 @@ const LeftNavBar: FC = () => { badgeColor={'primary'} />
    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) + } /> 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,