diff --git a/client/src/actions/login.js b/client/src/actions/login.js index d7304c27..afee55b7 100644 --- a/client/src/actions/login.js +++ b/client/src/actions/login.js @@ -28,20 +28,20 @@ authenticate.failure = (error) => ({ }, }); -const authenticateWithOidc = () => ({ - type: ActionTypes.WITH_OIDC_AUTHENTICATE, +const authenticateUsingOidc = () => ({ + type: ActionTypes.USING_OIDC_AUTHENTICATE, payload: {}, }); -authenticateWithOidc.success = (accessToken) => ({ - type: ActionTypes.WITH_OIDC_AUTHENTICATE__SUCCESS, +authenticateUsingOidc.success = (accessToken) => ({ + type: ActionTypes.USING_OIDC_AUTHENTICATE__SUCCESS, payload: { accessToken, }, }); -authenticateWithOidc.failure = (error) => ({ - type: ActionTypes.WITH_OIDC_AUTHENTICATE__FAILURE, +authenticateUsingOidc.failure = (error) => ({ + type: ActionTypes.USING_OIDC_AUTHENTICATE__FAILURE, payload: { error, }, @@ -55,6 +55,6 @@ const clearAuthenticateError = () => ({ export default { initializeLogin, authenticate, - authenticateWithOidc, + authenticateUsingOidc, clearAuthenticateError, }; diff --git a/client/src/api/access-tokens.js b/client/src/api/access-tokens.js index 2c538272..8c57500f 100755 --- a/client/src/api/access-tokens.js +++ b/client/src/api/access-tokens.js @@ -5,14 +5,14 @@ import socket from './socket'; const createAccessToken = (data, headers) => http.post('/access-tokens', data, headers); -const exchangeToAccessToken = (data, headers) => - http.post('/access-tokens/exchange', data, headers); +const exchangeForAccessTokenUsingOidc = (data, headers) => + http.post('/access-tokens/exchange-using-oidc', data, headers); const deleteCurrentAccessToken = (headers) => socket.delete('/access-tokens/me', undefined, headers); export default { createAccessToken, - exchangeToAccessToken, + exchangeForAccessTokenUsingOidc, deleteCurrentAccessToken, }; diff --git a/client/src/components/Login/Login.jsx b/client/src/components/Login/Login.jsx index 2b380a8e..84e6a9d0 100755 --- a/client/src/components/Login/Login.jsx +++ b/client/src/components/Login/Login.jsx @@ -65,11 +65,11 @@ const Login = React.memo( ({ defaultData, isSubmitting, - isSubmittingWithOidc, + isSubmittingUsingOidc, error, withOidc, onAuthenticate, - onAuthenticateWithOidc, + onAuthenticateUsingOidc, onMessageDismiss, }) => { const [t] = useTranslation(); @@ -192,15 +192,15 @@ const Login = React.memo( content={t('action.logIn')} floated="right" loading={isSubmitting} - disabled={isSubmitting || isSubmittingWithOidc} + disabled={isSubmitting || isSubmittingUsingOidc} /> {withOidc && ( @@ -239,11 +239,11 @@ Login.propTypes = { defaultData: PropTypes.object.isRequired, /* eslint-enable react/forbid-prop-types */ isSubmitting: PropTypes.bool.isRequired, - isSubmittingWithOidc: PropTypes.bool.isRequired, + isSubmittingUsingOidc: PropTypes.bool.isRequired, error: PropTypes.object, // eslint-disable-line react/forbid-prop-types withOidc: PropTypes.bool.isRequired, onAuthenticate: PropTypes.func.isRequired, - onAuthenticateWithOidc: PropTypes.func.isRequired, + onAuthenticateUsingOidc: PropTypes.func.isRequired, onMessageDismiss: PropTypes.func.isRequired, }; diff --git a/client/src/constants/ActionTypes.js b/client/src/constants/ActionTypes.js index 6793fad5..255160c3 100644 --- a/client/src/constants/ActionTypes.js +++ b/client/src/constants/ActionTypes.js @@ -16,9 +16,9 @@ export default { AUTHENTICATE: 'AUTHENTICATE', AUTHENTICATE__SUCCESS: 'AUTHENTICATE__SUCCESS', AUTHENTICATE__FAILURE: 'AUTHENTICATE__FAILURE', - WITH_OIDC_AUTHENTICATE: 'WITH_OIDC_AUTHENTICATE', - WITH_OIDC_AUTHENTICATE__SUCCESS: 'WITH_OIDC_AUTHENTICATE__SUCCESS', - WITH_OIDC_AUTHENTICATE__FAILURE: 'WITH_OIDC_AUTHENTICATE__FAILURE', + USING_OIDC_AUTHENTICATE: 'USING_OIDC_AUTHENTICATE', + USING_OIDC_AUTHENTICATE__SUCCESS: 'USING_OIDC_AUTHENTICATE__SUCCESS', + USING_OIDC_AUTHENTICATE__FAILURE: 'USING_OIDC_AUTHENTICATE__FAILURE', AUTHENTICATE_ERROR_CLEAR: 'AUTHENTICATE_ERROR_CLEAR', /* Core */ diff --git a/client/src/constants/EntryActionTypes.js b/client/src/constants/EntryActionTypes.js index 01468617..5a66c0fe 100755 --- a/client/src/constants/EntryActionTypes.js +++ b/client/src/constants/EntryActionTypes.js @@ -11,7 +11,7 @@ export default { /* Login */ AUTHENTICATE: `${PREFIX}/AUTHENTICATE`, - WITH_OIDC_AUTHENTICATE: `${PREFIX}/WITH_OIDC_AUTHENTICATE`, + USING_OIDC_AUTHENTICATE: `${PREFIX}/USING_OIDC_AUTHENTICATE`, AUTHENTICATE_ERROR_CLEAR: `${PREFIX}/AUTHENTICATE_ERROR_CLEAR`, /* Core */ diff --git a/client/src/containers/LoginContainer.js b/client/src/containers/LoginContainer.js index 34f15fdd..fc7ec217 100755 --- a/client/src/containers/LoginContainer.js +++ b/client/src/containers/LoginContainer.js @@ -10,14 +10,14 @@ const mapStateToProps = (state) => { const { ui: { - authenticateForm: { data: defaultData, isSubmitting, isSubmittingWithOidc, error }, + authenticateForm: { data: defaultData, isSubmitting, isSubmittingUsingOidc, error }, }, } = state; return { defaultData, isSubmitting, - isSubmittingWithOidc, + isSubmittingUsingOidc, error, withOidc: !!oidcConfig, }; @@ -27,7 +27,7 @@ const mapDispatchToProps = (dispatch) => bindActionCreators( { onAuthenticate: entryActions.authenticate, - onAuthenticateWithOidc: entryActions.authenticateWithOidc, + onAuthenticateUsingOidc: entryActions.authenticateUsingOidc, onMessageDismiss: entryActions.clearAuthenticateError, }, dispatch, diff --git a/client/src/entry-actions/login.js b/client/src/entry-actions/login.js index 89769217..8eb20475 100755 --- a/client/src/entry-actions/login.js +++ b/client/src/entry-actions/login.js @@ -7,8 +7,8 @@ const authenticate = (data) => ({ }, }); -const authenticateWithOidc = () => ({ - type: EntryActionTypes.WITH_OIDC_AUTHENTICATE, +const authenticateUsingOidc = () => ({ + type: EntryActionTypes.USING_OIDC_AUTHENTICATE, payload: {}, }); @@ -19,6 +19,6 @@ const clearAuthenticateError = () => ({ export default { authenticate, - authenticateWithOidc, + authenticateUsingOidc, clearAuthenticateError, }; diff --git a/client/src/reducers/auth.js b/client/src/reducers/auth.js index 8af35d46..d55821e4 100755 --- a/client/src/reducers/auth.js +++ b/client/src/reducers/auth.js @@ -10,7 +10,7 @@ const initialState = { export default (state = initialState, { type, payload }) => { switch (type) { case ActionTypes.AUTHENTICATE__SUCCESS: - case ActionTypes.WITH_OIDC_AUTHENTICATE__SUCCESS: + case ActionTypes.USING_OIDC_AUTHENTICATE__SUCCESS: return { ...state, accessToken: payload.accessToken, diff --git a/client/src/reducers/root.js b/client/src/reducers/root.js index d11c5846..c40be985 100644 --- a/client/src/reducers/root.js +++ b/client/src/reducers/root.js @@ -15,7 +15,7 @@ export default (state = initialState, { type, payload }) => { config: payload.config, }; case ActionTypes.AUTHENTICATE__SUCCESS: - case ActionTypes.WITH_OIDC_AUTHENTICATE__SUCCESS: + case ActionTypes.USING_OIDC_AUTHENTICATE__SUCCESS: return { ...state, isInitializing: true, diff --git a/client/src/reducers/ui/authenticate-form.js b/client/src/reducers/ui/authenticate-form.js index 4a0414c0..dfc1a443 100644 --- a/client/src/reducers/ui/authenticate-form.js +++ b/client/src/reducers/ui/authenticate-form.js @@ -9,7 +9,7 @@ const initialState = { password: '', }, isSubmitting: false, - isSubmittingWithOidc: false, + isSubmittingUsingOidc: false, error: null, }; @@ -20,7 +20,7 @@ export default (state = initialState, { type, payload }) => { if (payload.location.pathname === Paths.OIDC_CALLBACK) { return { ...state, - isSubmittingWithOidc: true, + isSubmittingUsingOidc: true, }; } @@ -35,7 +35,7 @@ export default (state = initialState, { type, payload }) => { isSubmitting: true, }; case ActionTypes.AUTHENTICATE__SUCCESS: - case ActionTypes.WITH_OIDC_AUTHENTICATE__SUCCESS: + case ActionTypes.USING_OIDC_AUTHENTICATE__SUCCESS: return initialState; case ActionTypes.AUTHENTICATE__FAILURE: return { @@ -43,10 +43,10 @@ export default (state = initialState, { type, payload }) => { isSubmitting: false, error: payload.error, }; - case ActionTypes.WITH_OIDC_AUTHENTICATE__FAILURE: + case ActionTypes.USING_OIDC_AUTHENTICATE__FAILURE: return { ...state, - isSubmittingWithOidc: false, + isSubmittingUsingOidc: false, error: payload.error, }; case ActionTypes.AUTHENTICATE_ERROR_CLEAR: diff --git a/client/src/sagas/core/index.js b/client/src/sagas/core/index.js index e841af98..391a8134 100755 --- a/client/src/sagas/core/index.js +++ b/client/src/sagas/core/index.js @@ -1,7 +1,8 @@ -import { all, apply, fork, take } from 'redux-saga/effects'; +import { all, apply, fork, select, take } from 'redux-saga/effects'; import watchers from './watchers'; import services from './services'; +import selectors from '../../selectors'; import { socket } from '../../api'; import ActionTypes from '../../constants/ActionTypes'; import Paths from '../../constants/Paths'; @@ -14,5 +15,12 @@ export default function* coreSaga() { yield take(ActionTypes.LOGOUT); - window.location.href = Paths.LOGIN; + const oidcConfig = yield select(selectors.selectOidcConfig); + + if (oidcConfig && oidcConfig.endSessionUrl !== null) { + // Redirect the user to the IDP to log out. + window.location.href = oidcConfig.endSessionUrl; + } else { + window.location.href = Paths.LOGIN; + } } diff --git a/client/src/sagas/core/services/core.js b/client/src/sagas/core/services/core.js index 2d7c96db..2dea7ea1 100644 --- a/client/src/sagas/core/services/core.js +++ b/client/src/sagas/core/services/core.js @@ -83,15 +83,7 @@ export function* logout(invalidateAccessToken = true) { } catch (error) {} // eslint-disable-line no-empty } - const oidcConfig = yield select(selectors.selectOidcConfig); - yield put(actions.logout()); - - if (oidcConfig && oidcConfig.endSessionUrl !== null) { - // Redirect the user to the IDP to log out. - window.location.replace(oidcConfig.endSessionUrl); - } - yield take(); } diff --git a/client/src/sagas/login/index.js b/client/src/sagas/login/index.js index f77fed9c..27ce09ef 100755 --- a/client/src/sagas/login/index.js +++ b/client/src/sagas/login/index.js @@ -9,7 +9,7 @@ export default function* loginSaga() { yield fork(services.initializeLogin); - yield take([ActionTypes.AUTHENTICATE__SUCCESS, ActionTypes.WITH_OIDC_AUTHENTICATE__SUCCESS]); + yield take([ActionTypes.AUTHENTICATE__SUCCESS, ActionTypes.USING_OIDC_AUTHENTICATE__SUCCESS]); yield cancel(watcherTasks); yield call(services.goToRoot); diff --git a/client/src/sagas/login/services/login.js b/client/src/sagas/login/services/login.js index 74574f18..1cb5b506 100644 --- a/client/src/sagas/login/services/login.js +++ b/client/src/sagas/login/services/login.js @@ -29,19 +29,22 @@ export function* authenticate(data) { yield put(actions.authenticate.success(accessToken)); } -export function* authenticateWithOidc() { +export function* authenticateUsingOidc() { const oidcConfig = yield select(selectors.selectOidcConfig); const nonce = nanoid(); window.sessionStorage.setItem('oidc-nonce', nonce); - window.location.replace(`${oidcConfig.authorizationUrl}&nonce=${encodeURIComponent(nonce)}`); + window.location.href = `${oidcConfig.authorizationUrl}&nonce=${encodeURIComponent(nonce)}`; } -export function* authenticateWithOidcCallback() { +export function* authenticateUsingOidcCallback() { const params = new URLSearchParams(window.location.hash.substring(1)); + + yield put(replace(Paths.LOGIN)); + if (params.get('error') !== null) { yield put( - actions.authenticateWithOidc.failure( + actions.authenticateUsingOidc.failure( new Error( `OIDC Authorization error: ${params.get('error')}: ${params.get('error_description')}`, ), @@ -53,7 +56,7 @@ export function* authenticateWithOidcCallback() { const nonce = window.sessionStorage.getItem('oidc-nonce'); if (nonce === null) { yield put( - actions.authenticateWithOidc.failure( + actions.authenticateUsingOidc.failure( new Error('Unable to process OIDC response: no nonce issued'), ), ); @@ -63,29 +66,27 @@ export function* authenticateWithOidcCallback() { const code = params.get('code'); if (code === null) { yield put( - actions.authenticateWithOidc.failure(new Error('Invalid OIDC response: no code parameter')), + actions.authenticateUsingOidc.failure(new Error('Invalid OIDC response: no code parameter')), ); return; } window.sessionStorage.removeItem('oidc-nonce'); - yield put(replace(Paths.LOGIN)); - if (code !== null) { let accessToken; try { - ({ item: accessToken } = yield call(api.exchangeToAccessToken, { + ({ item: accessToken } = yield call(api.exchangeForAccessTokenUsingOidc, { code, nonce, })); } catch (error) { - yield put(actions.authenticateWithOidc.failure(error)); + yield put(actions.authenticateUsingOidc.failure(error)); return; } yield call(setAccessToken, accessToken); - yield put(actions.authenticateWithOidc.success(accessToken)); + yield put(actions.authenticateUsingOidc.success(accessToken)); } } @@ -96,7 +97,7 @@ export function* clearAuthenticateError() { export default { initializeLogin, authenticate, - authenticateWithOidc, - authenticateWithOidcCallback, + authenticateUsingOidc, + authenticateUsingOidcCallback, clearAuthenticateError, }; diff --git a/client/src/sagas/login/services/router.js b/client/src/sagas/login/services/router.js index 45656728..70c8bfd7 100644 --- a/client/src/sagas/login/services/router.js +++ b/client/src/sagas/login/services/router.js @@ -1,7 +1,7 @@ import { call, put, select, take } from 'redux-saga/effects'; import { push } from '../../../lib/redux-router'; -import { authenticateWithOidcCallback } from './login'; +import { authenticateUsingOidcCallback } from './login'; import selectors from '../../../selectors'; import ActionTypes from '../../../constants/ActionTypes'; import Paths from '../../../constants/Paths'; @@ -36,9 +36,7 @@ export function* handleLocationChange() { yield take(ActionTypes.LOGIN_INITIALIZE); } - // TODO: check if OIDC is enabled - - yield call(authenticateWithOidcCallback); + yield call(authenticateUsingOidcCallback); break; } diff --git a/client/src/sagas/login/watchers/login.js b/client/src/sagas/login/watchers/login.js index 84aef934..89a53b21 100644 --- a/client/src/sagas/login/watchers/login.js +++ b/client/src/sagas/login/watchers/login.js @@ -8,7 +8,7 @@ export default function* loginWatchers() { takeEvery(EntryActionTypes.AUTHENTICATE, ({ payload: { data } }) => services.authenticate(data), ), - takeEvery(EntryActionTypes.WITH_OIDC_AUTHENTICATE, () => services.authenticateWithOidc()), + takeEvery(EntryActionTypes.USING_OIDC_AUTHENTICATE, () => services.authenticateUsingOidc()), takeEvery(EntryActionTypes.AUTHENTICATE_ERROR_CLEAR, () => services.clearAuthenticateError()), ]); } diff --git a/server/api/controllers/access-tokens/exchange.js b/server/api/controllers/access-tokens/exchange-using-oidc.js similarity index 81% rename from server/api/controllers/access-tokens/exchange.js rename to server/api/controllers/access-tokens/exchange-using-oidc.js index 582cd296..469d4462 100644 --- a/server/api/controllers/access-tokens/exchange.js +++ b/server/api/controllers/access-tokens/exchange-using-oidc.js @@ -1,8 +1,8 @@ const { getRemoteAddress } = require('../../../utils/remoteAddress'); const Errors = { - INVALID_TOKEN: { - invalidToken: 'Invalid token', + INVALID_CODE_OR_NONCE: { + invalidCodeOrNonce: 'Invalid code or nonce', }, EMAIL_ALREADY_IN_USE: { emailAlreadyInUse: 'Email already in use', @@ -28,7 +28,7 @@ module.exports = { }, exits: { - invalidToken: { + invalidCodeOrNonce: { responseType: 'unauthorized', }, emailAlreadyInUse: { @@ -46,10 +46,10 @@ module.exports = { const remoteAddress = getRemoteAddress(this.req); const user = await sails.helpers.users - .getOrCreateOneByOidcToken(inputs.code, inputs.nonce) - .intercept('invalidToken', () => { - sails.log.warn(`Invalid token! (IP: ${remoteAddress})`); - return Errors.INVALID_TOKEN; + .getOrCreateOneUsingOidc(inputs.code, inputs.nonce) + .intercept('invalidCodeOrNonce', () => { + sails.log.warn(`Invalid code or nonce! (IP: ${remoteAddress})`); + return Errors.INVALID_CODE_OR_NONCE; }) .intercept('emailAlreadyInUse', () => Errors.EMAIL_ALREADY_IN_USE) .intercept('usernameAlreadyInUse', () => Errors.USERNAME_ALREADY_IN_USE) diff --git a/server/api/controllers/show-config.js b/server/api/controllers/show-config.js index e41bb0fb..a580a7c0 100644 --- a/server/api/controllers/show-config.js +++ b/server/api/controllers/show-config.js @@ -1,20 +1,21 @@ module.exports = { fn() { - const oidcClient = sails.hooks.oidc.isActive() ? sails.hooks.oidc.getClient() : null; + let oidc = null; + if (sails.hooks.oidc.isActive()) { + const oidcClient = sails.hooks.oidc.getClient(); + + oidc = { + authorizationUrl: oidcClient.authorizationUrl({ + scope: sails.config.custom.oidcScopes, + response_mode: 'fragment', + }), + endSessionUrl: oidcClient.issuer.end_session_endpoint ? oidcClient.endSessionUrl({}) : null, + }; + } + return { item: { - oidc: - sails.config.custom.oidcIssuer !== '' - ? { - authorizationUrl: oidcClient.authorizationUrl({ - scope: sails.config.custom.oidcScopes, - response_mode: 'fragment', - }), - endSessionUrl: oidcClient.issuer.end_session_endpoint - ? oidcClient.endSessionUrl({}) - : null, - } - : null, + oidc, }, }; }, diff --git a/server/api/helpers/users/get-or-create-one-by-oidc-token.js b/server/api/helpers/users/get-or-create-one-using-oidc.js similarity index 95% rename from server/api/helpers/users/get-or-create-one-by-oidc-token.js rename to server/api/helpers/users/get-or-create-one-using-oidc.js index 50588dba..9c3278e1 100644 --- a/server/api/helpers/users/get-or-create-one-by-oidc-token.js +++ b/server/api/helpers/users/get-or-create-one-using-oidc.js @@ -11,7 +11,7 @@ module.exports = { }, exits: { - invalidToken: {}, + invalidCodeOrNonce: {}, missingValues: {}, emailAlreadyInUse: {}, usernameAlreadyInUse: {}, @@ -23,14 +23,14 @@ module.exports = { let userInfo; try { const tokenSet = await client.callback( - `${sails.config.custom.baseUrl}/oidc-callback`, + sails.config.custom.oidcRedirectUri, { code: inputs.code }, { nonce: inputs.nonce }, ); userInfo = await client.userinfo(tokenSet); } catch (e) { sails.log.warn(`Error while exchanging OIDC code: ${e}`); - throw 'invalidToken'; + throw 'invalidCodeOrNonce'; } if (!userInfo.email || !userInfo.name) { @@ -73,7 +73,7 @@ module.exports = { } else { // If no IDP/User mapping exists, search for the user by email. user = await sails.helpers.users.getOne({ - email: values.email, + email: values.email.toLowerCase(), }); // Otherwise, create a new user. diff --git a/server/api/hooks/oidc/index.js b/server/api/hooks/oidc/index.js index 9ed9b0a9..05d9d0bb 100644 --- a/server/api/hooks/oidc/index.js +++ b/server/api/hooks/oidc/index.js @@ -15,7 +15,7 @@ module.exports = function oidcServiceHook(sails) { client = new issuer.Client({ client_id: sails.config.custom.oidcClientId, client_secret: sails.config.custom.oidcClientSecret, - redirect_uris: [`${sails.config.custom.baseUrl}/oidc-callback`], + redirect_uris: [sails.config.custom.oidcRedirectUri], response_types: ['code'], }); sails.log.info('OIDC hook has been loaded successfully'); diff --git a/server/config/custom.js b/server/config/custom.js index 24a9e5b8..f79368cf 100644 --- a/server/config/custom.js +++ b/server/config/custom.js @@ -39,4 +39,9 @@ module.exports.custom = { oidcScopes: process.env.OIDC_SCOPES || 'openid email profile', oidcAdminRoles: process.env.OIDC_ADMIN_ROLES ? process.env.OIDC_ADMIN_ROLES.split(',') : [], oidcRolesAttribute: process.env.OIDC_ROLES_ATTRIBUTE || 'groups', + + // TODO: move client base url to environment variable? + oidcRedirectUri: `${ + sails.config.environment === 'production' ? process.env.BASE_URL : 'http://localhost:3000' + }/oidc-callback`, }; diff --git a/server/config/policies.js b/server/config/policies.js index 6d923403..665689df 100644 --- a/server/config/policies.js +++ b/server/config/policies.js @@ -25,5 +25,5 @@ module.exports.policies = { 'show-config': true, 'access-tokens/create': true, - 'access-tokens/exchange': true, + 'access-tokens/exchange-using-oidc': true, }; diff --git a/server/config/routes.js b/server/config/routes.js index 16ae350e..77f5c43b 100644 --- a/server/config/routes.js +++ b/server/config/routes.js @@ -12,7 +12,7 @@ module.exports.routes = { 'GET /api/config': 'show-config', 'POST /api/access-tokens': 'access-tokens/create', - 'POST /api/access-tokens/exchange': 'access-tokens/exchange', + 'POST /api/access-tokens/exchange-using-oidc': 'access-tokens/exchange-using-oidc', 'DELETE /api/access-tokens/me': 'access-tokens/delete', 'GET /api/users': 'users/index',