1
0
Fork 0
mirror of https://github.com/plankanban/planka.git synced 2025-07-18 20:59:44 +02:00

feat: Improve OIDC SSO (#524)

The OIDC implementation merged in https://github.com/plankanban/planka/pull/491 is flawed for multiple reasons.

It assumes that the access_token returned by the IDP has to be a JWT parseable by the RP which is not the case [1].
Many major IDPs do issue tokens which are not JWTs and RPs should not rely on the contents of these at all.
The only signed token which has a standardized format for direct RP consumption is the OIDC ID token (id_token), but this by default doesn't contain many claims, especially role claims are omitted from them by default for size reasons. To get these additional claims into the ID token, one needs an IDP with support for the "claims" parameter.

It requires manual specification of the JWKS URL which is mandatory in any OIDC discovery document and thus never needs to be manually specified.

It also makes the questionable decision to use a client-side code flow with PKCE where a normal code flow would be much more appropriate as all user data is processed in the backend which can securely hold a client secret (confidential client). This has far wider IDP support, is safer (due to direct involvement of the IDP in obtaining user information) and doesn't require working with ID tokens and claim parameters.

By using a server-side code flow we can also offload most complexity to the server alone, no longer requiring an additional OIDC library on the web client.

Also silent logout doesn't work on most IDPs for security reasons, one needs to actually redirect the user over to the IDP, which then prompts them once more if they actually want to log out.

This implementation should work with any OIDC-compliant IDP and even OAuth 2.0-only IDPs as long as they serve and OIDC discovery document.

[1] rfc-editor.org/rfc/rfc6749#section-5.1
This commit is contained in:
Lorenz Brun 2023-10-19 14:39:21 +02:00 committed by GitHub
parent ec9194dac7
commit 9011ee61da
14 changed files with 192 additions and 496 deletions

View file

@ -18,8 +18,8 @@
"js-cookie": "^3.0.1",
"jwt-decode": "^3.1.2",
"lodash": "^4.17.21",
"nanoid": "^5.0.2",
"node-sass": "^8.0.0",
"oidc-client-ts": "^2.3.0",
"photoswipe": "^5.3.3",
"prop-types": "^15.8.1",
"react": "^18.2.0",
@ -7190,11 +7190,6 @@
"node": ">= 8"
}
},
"node_modules/crypto-js": {
"version": "4.1.1",
"resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.1.1.tgz",
"integrity": "sha512-o2JlM7ydqd3Qk9CA0L4NL6mTzU2sdx96a+oOfPu8Mkl/PK51vSyoi8/rQ8NknZtk44vq15lmhAj9CIAGwgeWKw=="
},
"node_modules/crypto-random-string": {
"version": "2.0.0",
"resolved": "https://registry.npmjs.org/crypto-random-string/-/crypto-random-string-2.0.0.tgz",
@ -16413,14 +16408,20 @@
"integrity": "sha512-2ZTgtl0nJsO0KQCjEpxcIr5D+Yv90plTitZt9JBfQvVJDS5seMl3FOvsh3+9CoYWXf/1l5OaZzzF6nDm4cagaQ=="
},
"node_modules/nanoid": {
"version": "3.3.4",
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.4.tgz",
"integrity": "sha512-MqBkQh/OHTS2egovRtLk45wEyNXwF+cokD+1YPf9u5VfJiRdAiRwB2froX5Co9Rh20xs4siNPm8naNotSD6RBw==",
"version": "5.0.2",
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-5.0.2.tgz",
"integrity": "sha512-2ustYUX1R2rL/Br5B/FMhi8d5/QzvkJ912rBYxskcpu0myTHzSZfTr1LAS2Sm7jxRUObRrSBFoyzwAhL49aVSg==",
"funding": [
{
"type": "github",
"url": "https://github.com/sponsors/ai"
}
],
"bin": {
"nanoid": "bin/nanoid.cjs"
"nanoid": "bin/nanoid.js"
},
"engines": {
"node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1"
"node": "^18 || >=20"
}
},
"node_modules/natural-compare": {
@ -17041,18 +17042,6 @@
"resolved": "https://registry.npmjs.org/obuf/-/obuf-1.1.2.tgz",
"integrity": "sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg=="
},
"node_modules/oidc-client-ts": {
"version": "2.3.0",
"resolved": "https://registry.npmjs.org/oidc-client-ts/-/oidc-client-ts-2.3.0.tgz",
"integrity": "sha512-7RUKU+TJFQo+4X9R50IGJAIDF18uRBaFXyZn4VVCfwmwbSUhKcdDnw4zgeut3uEXkiD3NqURq+d88sDPxjf1FA==",
"dependencies": {
"crypto-js": "^4.1.1",
"jwt-decode": "^3.1.2"
},
"engines": {
"node": ">=12.13.0"
}
},
"node_modules/on-finished": {
"version": "2.4.1",
"resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.4.1.tgz",
@ -18676,6 +18665,23 @@
"resolved": "https://registry.npmjs.org/postcss-value-parser/-/postcss-value-parser-4.2.0.tgz",
"integrity": "sha512-1NNCs6uurfkVbeXG4S8JFT9t19m45ICnif8zWLd5oPSZ50QnwMfK+H3jv408d4jw/7Bttv5axS5IiHoLaVNHeQ=="
},
"node_modules/postcss/node_modules/nanoid": {
"version": "3.3.6",
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.6.tgz",
"integrity": "sha512-BGcqMMJuToF7i1rt+2PWSNVnWIkGCU78jBG3RxO/bZlnZPK2Cmi2QaffxGO/2RvWi9sL+FAiRiXMgsyxQ1DIDA==",
"funding": [
{
"type": "github",
"url": "https://github.com/sponsors/ai"
}
],
"bin": {
"nanoid": "bin/nanoid.cjs"
},
"engines": {
"node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1"
}
},
"node_modules/prelude-ls": {
"version": "1.2.1",
"resolved": "https://registry.npmjs.org/prelude-ls/-/prelude-ls-1.2.1.tgz",
@ -28785,11 +28791,6 @@
"which": "^2.0.1"
}
},
"crypto-js": {
"version": "4.1.1",
"resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.1.1.tgz",
"integrity": "sha512-o2JlM7ydqd3Qk9CA0L4NL6mTzU2sdx96a+oOfPu8Mkl/PK51vSyoi8/rQ8NknZtk44vq15lmhAj9CIAGwgeWKw=="
},
"crypto-random-string": {
"version": "2.0.0",
"resolved": "https://registry.npmjs.org/crypto-random-string/-/crypto-random-string-2.0.0.tgz",
@ -35494,9 +35495,9 @@
"integrity": "sha512-2ZTgtl0nJsO0KQCjEpxcIr5D+Yv90plTitZt9JBfQvVJDS5seMl3FOvsh3+9CoYWXf/1l5OaZzzF6nDm4cagaQ=="
},
"nanoid": {
"version": "3.3.4",
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.4.tgz",
"integrity": "sha512-MqBkQh/OHTS2egovRtLk45wEyNXwF+cokD+1YPf9u5VfJiRdAiRwB2froX5Co9Rh20xs4siNPm8naNotSD6RBw=="
"version": "5.0.2",
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-5.0.2.tgz",
"integrity": "sha512-2ustYUX1R2rL/Br5B/FMhi8d5/QzvkJ912rBYxskcpu0myTHzSZfTr1LAS2Sm7jxRUObRrSBFoyzwAhL49aVSg=="
},
"natural-compare": {
"version": "1.4.0",
@ -35956,15 +35957,6 @@
"resolved": "https://registry.npmjs.org/obuf/-/obuf-1.1.2.tgz",
"integrity": "sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg=="
},
"oidc-client-ts": {
"version": "2.3.0",
"resolved": "https://registry.npmjs.org/oidc-client-ts/-/oidc-client-ts-2.3.0.tgz",
"integrity": "sha512-7RUKU+TJFQo+4X9R50IGJAIDF18uRBaFXyZn4VVCfwmwbSUhKcdDnw4zgeut3uEXkiD3NqURq+d88sDPxjf1FA==",
"requires": {
"crypto-js": "^4.1.1",
"jwt-decode": "^3.1.2"
}
},
"on-finished": {
"version": "2.4.1",
"resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.4.1.tgz",
@ -36284,6 +36276,13 @@
"nanoid": "^3.3.4",
"picocolors": "^1.0.0",
"source-map-js": "^1.0.2"
},
"dependencies": {
"nanoid": {
"version": "3.3.6",
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.6.tgz",
"integrity": "sha512-BGcqMMJuToF7i1rt+2PWSNVnWIkGCU78jBG3RxO/bZlnZPK2Cmi2QaffxGO/2RvWi9sL+FAiRiXMgsyxQ1DIDA=="
}
}
},
"postcss-attribute-case-insensitive": {

View file

@ -65,8 +65,8 @@
"js-cookie": "^3.0.1",
"jwt-decode": "^3.1.2",
"lodash": "^4.17.21",
"nanoid": "^5.0.2",
"node-sass": "^8.0.0",
"oidc-client-ts": "^2.3.0",
"photoswipe": "^5.3.3",
"prop-types": "^15.8.1",
"react": "^18.2.0",

View file

@ -1,4 +1,4 @@
import { apply, call, put, select, take } from 'redux-saga/effects';
import { call, put, select, take } from 'redux-saga/effects';
import request from '../request';
import requests from '../requests';
@ -6,7 +6,6 @@ import selectors from '../../../selectors';
import actions from '../../../actions';
import api from '../../../api';
import i18n from '../../../i18n';
import { createOidcManager } from '../../../utils/oidc-manager';
import { removeAccessToken } from '../../../utils/access-token-storage';
export function* initializeCore() {
@ -86,15 +85,13 @@ export function* logout(invalidateAccessToken = true) {
const oidcConfig = yield select(selectors.selectOidcConfig);
if (oidcConfig) {
const oidcManager = createOidcManager(oidcConfig);
yield put(actions.logout());
try {
yield apply(oidcManager, oidcManager.logout);
} catch (error) {} // eslint-disable-line no-empty
if (oidcConfig && oidcConfig.endSessionUrl !== null) {
// Redirect the user to the IDP to log out.
window.location.replace(oidcConfig.endSessionUrl);
}
yield put(actions.logout());
yield take();
}

View file

@ -1,10 +1,10 @@
import { apply, call, put, select } from 'redux-saga/effects';
import { nanoid } from 'nanoid';
import { call, put, select } from 'redux-saga/effects';
import { replace } from '../../../lib/redux-router';
import selectors from '../../../selectors';
import actions from '../../../actions';
import api from '../../../api';
import { createOidcManager } from '../../../utils/oidc-manager';
import { setAccessToken } from '../../../utils/access-token-storage';
import Paths from '../../../constants/Paths';
@ -31,29 +31,53 @@ export function* authenticate(data) {
export function* authenticateWithOidc() {
const oidcConfig = yield select(selectors.selectOidcConfig);
const oidcManager = createOidcManager(oidcConfig);
yield apply(oidcManager, oidcManager.login);
const nonce = nanoid();
window.sessionStorage.setItem('oidc-nonce', nonce);
window.location.replace(`${oidcConfig.authorizationUrl}&nonce=${encodeURIComponent(nonce)}`);
}
export function* authenticateWithOidcCallback() {
const oidcConfig = yield select(selectors.selectOidcConfig);
const oidcManager = createOidcManager(oidcConfig);
let oidcToken;
try {
({ access_token: oidcToken } = yield apply(oidcManager, oidcManager.loginCallback));
} catch (error) {
yield put(actions.authenticateWithOidc.failure(error));
const params = new URLSearchParams(window.location.hash.substring(1));
if (params.get('error') !== null) {
yield put(
actions.authenticateWithOidc.failure(
new Error(
`OIDC Authorization error: ${params.get('error')}: ${params.get('error_description')}`,
),
),
);
return;
}
const nonce = window.sessionStorage.getItem('oidc-nonce');
if (nonce === null) {
yield put(
actions.authenticateWithOidc.failure(
new Error('Unable to process OIDC response: no nonce issued'),
),
);
return;
}
const code = params.get('code');
if (code === null) {
yield put(
actions.authenticateWithOidc.failure(new Error('Invalid OIDC response: no code parameter')),
);
return;
}
window.sessionStorage.removeItem('oidc-nonce');
yield put(replace(Paths.LOGIN));
if (oidcToken) {
if (code !== null) {
let accessToken;
try {
({ item: accessToken } = yield call(api.exchangeToAccessToken, {
token: oidcToken,
code,
nonce,
}));
} catch (error) {
yield put(actions.authenticateWithOidc.failure(error));

View file

@ -1,27 +0,0 @@
import { UserManager } from 'oidc-client-ts';
export default class OidcManager {
constructor(config) {
this.userManager = new UserManager({
...config,
authority: config.issuer,
client_id: config.clientId,
redirect_uri: config.redirectUri,
scope: config.scopes,
});
}
login() {
return this.userManager.signinRedirect();
}
loginCallback() {
return this.userManager.signinRedirectCallback();
}
logout() {
return this.userManager.signoutSilent();
}
}
export const createOidcManager = (config) => new OidcManager(config);