Archives
/
gristlabs_grist-core
mirror of https://github.com/gristlabs/grist-core
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Add friendly page for login failure

Browse Source
fflorent 3 months ago
parent 1bf114a38b
commit 0115474fa6
4 changed files with 82 additions and 32 deletions
Show Stats Download Patch File Download Diff File

14
app/client/ui/errorPages.ts
Unescape View File

@ -21,6 +21,7 @@ export function createErrPage(appModel: AppModel) {
errPage === 'not-found' ? createNotFoundPage(appModel, errMessage) :
errPage === 'access-denied' ? createForbiddenPage(appModel, errMessage) :
errPage === 'account-deleted' ? createAccountDeletedPage(appModel) :
errPage === 'signin-failed' ? createSigninFailedPage(appModel, errMessage) :
createOtherErrorPage(appModel, errMessage);
}
@ -98,6 +99,19 @@ export function createNotFoundPage(appModel: AppModel, message?: string) {
]);
}
export function createSigninFailedPage(appModel: AppModel, message?: string) {
document.title = t("Signin failed{{suffix}}", {suffix: getPageTitleSuffix(getGristConfig())});
return pagePanelsError(appModel, t("Signin failed{{suffix}}", {suffix: ''}), [
cssErrorText(message ??
t("Failed to login.{{separator}}Please try again or contact the support.", {
separator: dom('br')
})),
cssButtonWrap(bigPrimaryButtonLink(t("Login again"), testId('error-primary-btn'),
urlState().setLinkUrl({login: 'login', params: {}}))),
cssButtonWrap(bigBasicButtonLink(t("Contact support"), {href: commonUrls.contactSupport})),
]);
}
/**
* Creates a generic error page with the given message.
*/

36
app/server/lib/OIDCConfig.ts
Unescape View File

@ -70,6 +70,7 @@ import { RequestWithLogin } from './Authorizer';
import { UserProfile } from 'app/common/LoginSessionAPI';
import _ from 'lodash';
import { SessionObj } from './BrowserSession';
import { SendAppPage } from './sendAppPage';
enum ENABLED_PROTECTIONS {
NONCE,
@ -90,12 +91,21 @@ function formatTokenForLogs(token: TokenSet) {
}).value();
}
const DEFAULT_USER_FRIENDLY_MESSAGE =
"Something went wrong while logging, please try again or contact your administrator if the problem persists";
class ErrorWithUserFriendlyMessage extends Error {
constructor(errMessage: string, public readonly userFriendlyMessage: string = DEFAULT_USER_FRIENDLY_MESSAGE) {
super(errMessage);
}
}
export class OIDCConfig {
/**
* Handy alias to create an OIDCConfig instance and initialize it.
*/
public static async build(): Promise<OIDCConfig> {
const config = new OIDCConfig();
public static async build(sendAppPage: SendAppPage): Promise<OIDCConfig> {
const config = new OIDCConfig(sendAppPage);
await config.initOIDC();
return config;
}
@ -110,8 +120,9 @@ export class OIDCConfig {
private _enabledProtections: EnabledProtectionsString[] = [];
private _acrValues?: string;
protected constructor() {
}
protected constructor(
private _sendAppPage: SendAppPage
) {}
public async initOIDC(): Promise<void> {
const section = appSettings.section('login').section('system').section('oidc');
@ -195,7 +206,11 @@ export class OIDCConfig {
log.debug("Got userinfo: %o", userInfo);
if (!this._ignoreEmailVerified && userInfo.email_verified !== true) {
throw new Error(`OIDCConfig: email not verified for ${userInfo.email}`);
throw new ErrorWithUserFriendlyMessage(
`OIDCConfig: email not verified for ${userInfo.email}`,
"Your email is not verified according to the identity provider, please take the neccessary steps for that " +
"and log in again."
);
}
const profile = this._makeUserProfileFromUserInfo(userInfo);
@ -221,7 +236,14 @@ export class OIDCConfig {
//
// Also session deletion must be done before sending the response.
delete mreq.session.oidc;
res.status(500).send('OIDC callback failed.');
await this._sendAppPage(req, res, {
path: 'error.html',
status: 500,
config: {
errPage: 'signin-failed',
errMessage: err.userFriendlyMessage
},
});
}
}
@ -358,7 +380,7 @@ export async function getOIDCLoginSystem(): Promise<GristLoginSystem | undefined
if (!process.env.GRIST_OIDC_IDP_ISSUER) { return undefined; }
return {
async getMiddleware(gristServer: GristServer) {
const config = await OIDCConfig.build();
const config = await OIDCConfig.build(gristServer.sendAppPage.bind(gristServer));
return {
getLoginRedirectUrl: config.getLoginRedirectUrl.bind(config),
getSignUpRedirectUrl: config.getLoginRedirectUrl.bind(config),

12
app/server/lib/sendAppPage.ts
Unescape View File

@ -117,15 +117,15 @@ export function makeMessagePage(staticDir: string) {
};
}
export type SendAppPage = (req: express.Request, resp: express.Response, options: ISendAppPageOptions) => Promise<void>;
/**
* Send a simple template page, read from file at pagePath (relative to static/), with certain
* placeholders replaced.
*/
export function makeSendAppPage(opts: {
server: GristServer, staticDir: string, tag: string, testLogin?: boolean,
baseDomain?: string
}) {
const {server, staticDir, tag, testLogin} = opts;
export function makeSendAppPage({ server, staticDir, tag, testLogin, baseDomain }: {
server: GristServer, staticDir: string, tag: string, testLogin?: boolean, baseDomain?: string
}): SendAppPage {
// If env var GRIST_INCLUDE_CUSTOM_SCRIPT_URL is set, load it in a <script> tag on all app pages.
const customScriptUrl = process.env.GRIST_INCLUDE_CUSTOM_SCRIPT_URL;
@ -136,7 +136,7 @@ export function makeSendAppPage(opts: {
const config = makeGristConfig({
homeUrl: !isSingleUserMode() ? server.getHomeUrl(req) : null,
extra: options.config,
baseDomain: opts.baseDomain,
baseDomain: baseDomain,
req,
server,
});

52
test/server/lib/OIDCConfig.ts
Unescape View File

@ -9,10 +9,16 @@ import {Client, generators} from "openid-client";
import express from "express";
import _ from "lodash";
import {RequestWithLogin} from "app/server/lib/Authorizer";
import { SendAppPage } from "app/server/lib/sendAppPage";
const NOOPED_SEND_APP_PAGE: SendAppPage = () => Promise.resolve();
class OIDCConfigStubbed extends OIDCConfig {
public static async build(clientStub?: Client): Promise<OIDCConfigStubbed> {
const result = new OIDCConfigStubbed();
public static async buildWithStub(client: Client = new ClientStub().asClient()) {
return this.build(NOOPED_SEND_APP_PAGE, client);
}
public static async build(sendAppPage: SendAppPage, clientStub?: Client): Promise<OIDCConfigStubbed> {
const result = new OIDCConfigStubbed(sendAppPage);
if (clientStub) {
result._initClient = Sinon.spy(() => {
result._client = clientStub!;
@ -95,7 +101,7 @@ describe('OIDCConfig', () => {
]) {
setEnvVars();
delete process.env[envVar];
const promise = OIDCConfig.build();
const promise = OIDCConfig.build(NOOPED_SEND_APP_PAGE);
await assert.isRejected(promise, `missing environment variable: ${envVar}`);
}
});
@ -103,14 +109,14 @@ describe('OIDCConfig', () => {
it('should reject when the client initialization fails', async () => {
setEnvVars();
sandbox.stub(OIDCConfigStubbed.prototype, '_initClient').rejects(new Error('client init failed'));
const promise = OIDCConfigStubbed.build();
const promise = OIDCConfigStubbed.build(NOOPED_SEND_APP_PAGE);
await assert.isRejected(promise, 'client init failed');
});
it('should create a client with passed information', async () => {
setEnvVars();
const client = new ClientStub();
const config = await OIDCConfigStubbed.build(client.asClient());
const config = await OIDCConfigStubbed.buildWithStub(client.asClient());
assert.isTrue(config._initClient.calledOnce);
assert.deepEqual(config._initClient.firstCall.args, [{
clientId: process.env.GRIST_OIDC_IDP_CLIENT_ID,
@ -132,7 +138,7 @@ describe('OIDCConfig', () => {
};
process.env.GRIST_OIDC_IDP_EXTRA_CLIENT_METADATA = JSON.stringify(extraMetadata);
const client = new ClientStub();
const config = await OIDCConfigStubbed.build(client.asClient());
const config = await OIDCConfigStubbed.buildWithStub(client.asClient());
assert.isTrue(config._initClient.calledOnce);
assert.deepEqual(config._initClient.firstCall.args, [{
clientId: process.env.GRIST_OIDC_IDP_CLIENT_ID,
@ -182,7 +188,7 @@ describe('OIDCConfig', () => {
Object.assign(process.env, ctx.env);
const client = new ClientStub();
client.issuer.metadata.end_session_endpoint = ctx.end_session_endpoint;
const promise = OIDCConfigStubbed.build(client.asClient());
const promise = OIDCConfigStubbed.buildWithStub(client.asClient());
if (ctx.errorMsg) {
await assert.isRejected(promise, ctx.errorMsg);
assert.isFalse(logInfoStub.calledOnce);
@ -199,14 +205,14 @@ describe('OIDCConfig', () => {
it('should throw when GRIST_OIDC_IDP_ENABLED_PROTECTIONS contains unsupported values', async () => {
setEnvVars();
process.env.GRIST_OIDC_IDP_ENABLED_PROTECTIONS = 'STATE,NONCE,PKCE,invalid';
const promise = OIDCConfig.build();
const promise = OIDCConfig.build(NOOPED_SEND_APP_PAGE);
await assert.isRejected(promise, 'OIDC: Invalid protection in GRIST_OIDC_IDP_ENABLED_PROTECTIONS: invalid');
});
it('should successfully change the supported protections', async function () {
setEnvVars();
process.env.GRIST_OIDC_IDP_ENABLED_PROTECTIONS = 'NONCE';
const config = await OIDCConfigStubbed.build((new ClientStub()).asClient());
const config = await OIDCConfigStubbed.buildWithStub();
assert.isTrue(config.supportsProtection("NONCE"));
assert.isFalse(config.supportsProtection("PKCE"));
assert.isFalse(config.supportsProtection("STATE"));
@ -215,7 +221,7 @@ describe('OIDCConfig', () => {
it('should successfully accept an empty string', async function () {
setEnvVars();
process.env.GRIST_OIDC_IDP_ENABLED_PROTECTIONS = '';
const config = await OIDCConfigStubbed.build((new ClientStub()).asClient());
const config = await OIDCConfigStubbed.buildWithStub();
assert.isFalse(config.supportsProtection("NONCE"));
assert.isFalse(config.supportsProtection("PKCE"));
assert.isFalse(config.supportsProtection("STATE"));
@ -223,7 +229,7 @@ describe('OIDCConfig', () => {
it('if omitted, should defaults to "STATE,PKCE"', async function () {
setEnvVars();
const config = await OIDCConfigStubbed.build((new ClientStub()).asClient());
const config = await OIDCConfigStubbed.buildWithStub();
assert.isFalse(config.supportsProtection("NONCE"));
assert.isTrue(config.supportsProtection("PKCE"));
assert.isTrue(config.supportsProtection("STATE"));
@ -328,7 +334,7 @@ describe('OIDCConfig', () => {
setEnvVars();
Object.assign(process.env, ctx.env);
const clientStub = new ClientStub();
const config = await OIDCConfigStubbed.build(clientStub.asClient());
const config = await OIDCConfigStubbed.buildWithStub(clientStub.asClient());
const session = {};
const req = {
session
@ -474,6 +480,9 @@ describe('OIDCConfig', () => {
email_verified: false,
},
expectedErrorMsg: /email not verified for/,
extraChecks: function ({ sendAppPageStub }: { sendAppPageStub: Sinon.SinonStub }) {
assert.match(sendAppPageStub.firstCall.args[2].config.errMessage, /Your email is not verified/);
}
},
{
itMsg: 'should resolve when the userinfo mail is not verified but its check disabled',
@ -585,10 +594,11 @@ describe('OIDCConfig', () => {
setEnvVars();
Object.assign(process.env, ctx.env);
const clientStub = new ClientStub();
const sendAppPageStub = Sinon.stub().resolves();
const fakeParams = {
state: FAKE_STATE,
};
const config = await OIDCConfigStubbed.build(clientStub.asClient());
const config = await OIDCConfigStubbed.build(sendAppPageStub as SendAppPage, clientStub.asClient());
const session = _.clone(ctx.session); // session is modified, so clone it
const req = {
session,
@ -613,8 +623,11 @@ describe('OIDCConfig', () => {
if (ctx.expectedErrorMsg) {
assert.isTrue(logErrorStub.calledOnce);
assert.match(logErrorStub.firstCall.args[0], ctx.expectedErrorMsg);
assert.isTrue(fakeRes.status.calledOnceWith(500));
assert.isTrue(fakeRes.send.calledOnceWith('OIDC callback failed.'));
assert.isTrue(sendAppPageStub.calledOnceWith(req, fakeRes));
assert.include(sendAppPageStub.firstCall.args[2], {
path: 'error.html',
status: 500,
});
} else {
assert.isFalse(logErrorStub.called, 'no error should be logged. Got: ' + logErrorStub.firstCall?.args[0]);
assert.isTrue(fakeRes.redirect.calledOnce, 'should redirect');
@ -630,8 +643,8 @@ describe('OIDCConfig', () => {
idToken: tokenSet.id_token,
}
}, 'oidc info should only keep state and id_token in the session and for the logout');
ctx.extraChecks?.({fakeRes, user});
}
ctx.extraChecks?.({ fakeRes, user, sendAppPageStub });
});
});
@ -639,7 +652,8 @@ describe('OIDCConfig', () => {
// See https://github.com/panva/node-openid-client/blob/47a549cb4e36ffe2ebfe2dc9d6b69a02643cc0a9/lib/client.js#L1293
setEnvVars();
const clientStub = new ClientStub();
const config = await OIDCConfigStubbed.build(clientStub.asClient());
const sendAppPageStub = Sinon.stub().resolves();
const config = await OIDCConfigStubbed.build(sendAppPageStub, clientStub.asClient());
const req = {
session: DEFAULT_SESSION,
query: {
@ -662,7 +676,7 @@ describe('OIDCConfig', () => {
assert.isTrue(logErrorStub.calledTwice);
assert.include(logErrorStub.firstCall.args[0], err.message);
assert.include(logErrorStub.secondCall.args[0], err.response.body);
assert.isTrue(fakeRes.status.calledOnceWith(500));
assert.isTrue(sendAppPageStub.calledOnce, "An error should have been sent");
});
});
@ -705,7 +719,7 @@ describe('OIDCConfig', () => {
Object.assign(process.env, ctx.env);
const clientStub = new ClientStub();
clientStub.endSessionUrl.returns(URL_RETURNED_BY_CLIENT);
const config = await OIDCConfigStubbed.build(clientStub.asClient());
const config = await OIDCConfigStubbed.buildWithStub(clientStub.asClient());
const req = {
session: FAKE_SESSION
} as unknown as RequestWithLogin;

Write Preview
Loading…
Cancel
Save