Merge branch 'master' into feature/gitlab-auth-provider
Signed-off-by: Tom Opdebeeck <tom.opdebeeck@studiohyperdrive.be>
This commit is contained in:
@@ -64,6 +64,7 @@ describe('OAuthAdapter', () => {
|
||||
issueToken: async () => 'my-id-token',
|
||||
listPublicKeys: async () => ({ keys: [] }),
|
||||
},
|
||||
isOriginAllowed: () => false,
|
||||
};
|
||||
|
||||
it('sets the correct headers in start', async () => {
|
||||
@@ -105,6 +106,7 @@ describe('OAuthAdapter', () => {
|
||||
const oauthProvider = new OAuthAdapter(providerInstance, {
|
||||
...oAuthProviderOptions,
|
||||
disableRefresh: false,
|
||||
isOriginAllowed: () => false,
|
||||
});
|
||||
|
||||
const state = { nonce: 'nonce', env: 'development' };
|
||||
@@ -139,6 +141,7 @@ describe('OAuthAdapter', () => {
|
||||
const oauthProvider = new OAuthAdapter(providerInstance, {
|
||||
...oAuthProviderOptions,
|
||||
disableRefresh: true,
|
||||
isOriginAllowed: () => false,
|
||||
});
|
||||
|
||||
const mockRequest = {
|
||||
@@ -164,6 +167,7 @@ describe('OAuthAdapter', () => {
|
||||
const oauthProvider = new OAuthAdapter(providerInstance, {
|
||||
...oAuthProviderOptions,
|
||||
disableRefresh: false,
|
||||
isOriginAllowed: () => false,
|
||||
});
|
||||
|
||||
const mockRequest = {
|
||||
@@ -190,6 +194,7 @@ describe('OAuthAdapter', () => {
|
||||
const oauthProvider = new OAuthAdapter(providerInstance, {
|
||||
...oAuthProviderOptions,
|
||||
disableRefresh: false,
|
||||
isOriginAllowed: () => false,
|
||||
});
|
||||
|
||||
const mockRequest = {
|
||||
@@ -220,6 +225,7 @@ describe('OAuthAdapter', () => {
|
||||
const oauthProvider = new OAuthAdapter(providerInstance, {
|
||||
...oAuthProviderOptions,
|
||||
disableRefresh: true,
|
||||
isOriginAllowed: () => false,
|
||||
});
|
||||
|
||||
const mockRequest = {
|
||||
|
||||
@@ -22,11 +22,16 @@ import {
|
||||
BackstageIdentity,
|
||||
AuthProviderConfig,
|
||||
} from '../../providers/types';
|
||||
import { InputError } from '@backstage/errors';
|
||||
import { InputError, NotAllowedError } from '@backstage/errors';
|
||||
import { TokenIssuer } from '../../identity/types';
|
||||
import { verifyNonce } from './helpers';
|
||||
import { readState, verifyNonce } from './helpers';
|
||||
import { postMessageResponse, ensuresXRequestedWith } from '../flow';
|
||||
import { OAuthHandlers, OAuthStartRequest, OAuthRefreshRequest } from './types';
|
||||
import {
|
||||
OAuthHandlers,
|
||||
OAuthStartRequest,
|
||||
OAuthRefreshRequest,
|
||||
OAuthState,
|
||||
} from './types';
|
||||
|
||||
export const THOUSAND_DAYS_MS = 1000 * 24 * 60 * 60 * 1000;
|
||||
export const TEN_MINUTES_MS = 600 * 1000;
|
||||
@@ -40,6 +45,7 @@ export type Options = {
|
||||
cookiePath: string;
|
||||
appOrigin: string;
|
||||
tokenIssuer: TokenIssuer;
|
||||
isOriginAllowed: (origin: string) => boolean;
|
||||
};
|
||||
|
||||
export class OAuthAdapter implements AuthProviderRouteHandlers {
|
||||
@@ -61,6 +67,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
|
||||
cookieDomain: url.hostname,
|
||||
cookiePath,
|
||||
secure,
|
||||
isOriginAllowed: config.isOriginAllowed,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -73,6 +80,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
|
||||
// retrieve scopes from request
|
||||
const scope = req.query.scope?.toString() ?? '';
|
||||
const env = req.query.env?.toString();
|
||||
const origin = req.query.origin?.toString();
|
||||
|
||||
if (!env) {
|
||||
throw new InputError('No env provided in request query parameters');
|
||||
@@ -86,7 +94,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
|
||||
// set a nonce cookie before redirecting to oauth provider
|
||||
this.setNonceCookie(res, nonce);
|
||||
|
||||
const state = { nonce: nonce, env: env };
|
||||
const state = { nonce, env, origin };
|
||||
const forwardReq = Object.assign(req, { scope, state });
|
||||
|
||||
const { url, status } = await this.handlers.start(
|
||||
@@ -103,7 +111,22 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
|
||||
req: express.Request,
|
||||
res: express.Response,
|
||||
): Promise<void> {
|
||||
let appOrigin = this.options.appOrigin;
|
||||
|
||||
try {
|
||||
const state: OAuthState = readState(req.query.state?.toString() ?? '');
|
||||
|
||||
if (state.origin) {
|
||||
try {
|
||||
appOrigin = new URL(state.origin).origin;
|
||||
} catch {
|
||||
throw new NotAllowedError('App origin is invalid, failed to parse');
|
||||
}
|
||||
if (!this.options.isOriginAllowed(appOrigin)) {
|
||||
throw new NotAllowedError(`Origin '${appOrigin}' is not allowed`);
|
||||
}
|
||||
}
|
||||
|
||||
// verify nonce cookie and state cookie on callback
|
||||
verifyNonce(req, this.options.providerId);
|
||||
|
||||
@@ -129,13 +152,13 @@ export class OAuthAdapter implements AuthProviderRouteHandlers {
|
||||
await this.populateIdentity(response.backstageIdentity);
|
||||
|
||||
// post message back to popup if successful
|
||||
return postMessageResponse(res, this.options.appOrigin, {
|
||||
return postMessageResponse(res, appOrigin, {
|
||||
type: 'authorization_response',
|
||||
response,
|
||||
});
|
||||
} catch (error) {
|
||||
// post error message back to popup if failure
|
||||
return postMessageResponse(res, this.options.appOrigin, {
|
||||
return postMessageResponse(res, appOrigin, {
|
||||
type: 'authorization_response',
|
||||
error: {
|
||||
name: error.name,
|
||||
|
||||
@@ -77,6 +77,7 @@ export type OAuthState = {
|
||||
*/
|
||||
nonce: string;
|
||||
env: string;
|
||||
origin?: string;
|
||||
};
|
||||
|
||||
export type OAuthStartRequest = express.Request<{}> & {
|
||||
|
||||
@@ -34,6 +34,11 @@ export type AuthProviderConfig = {
|
||||
* The base URL of the app as provided by app.baseUrl
|
||||
*/
|
||||
appUrl: string;
|
||||
|
||||
/**
|
||||
* A function that is called to check whether an origin other than the apps default one is allowed.
|
||||
*/
|
||||
isOriginAllowed: (origin: string) => boolean;
|
||||
};
|
||||
|
||||
export type RedirectInfo = {
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
/*
|
||||
* Copyright 2020 The Backstage Authors
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
import { ConfigReader } from '@backstage/config';
|
||||
import { createOriginFilter } from './router';
|
||||
|
||||
describe('Auth origin filtering', () => {
|
||||
const defaultConfigOptions = {
|
||||
auth: {
|
||||
experimentalExtraAllowedOrigins: ['https://test-*.example.net'],
|
||||
},
|
||||
};
|
||||
const defaultConfig = () => new ConfigReader(defaultConfigOptions);
|
||||
const getOptionalString = jest.fn();
|
||||
const config = defaultConfig();
|
||||
config.getOptionalString = getOptionalString;
|
||||
it('Will explode, invalid origin', () => {
|
||||
const origin = 'https://test.example.net';
|
||||
expect(createOriginFilter(config)(origin)).toBeFalsy();
|
||||
});
|
||||
it('Will explode, invalid origin domain', () => {
|
||||
const origin = 'https://test-1234.examplee.net';
|
||||
expect(createOriginFilter(config)(origin)).toBeFalsy();
|
||||
});
|
||||
it("Won't explode, valid origin with numbers", () => {
|
||||
const origin = 'https://test-1234.example.net';
|
||||
expect(createOriginFilter(config)(origin)).toBeTruthy();
|
||||
});
|
||||
it("Won't explode, valid origin with chars and numbers", () => {
|
||||
const origin = 'https://test-test1234.example.net';
|
||||
expect(createOriginFilter(config)(origin)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
@@ -32,6 +32,7 @@ import { Config } from '@backstage/config';
|
||||
import { createOidcRouter, DatabaseKeyStore, TokenFactory } from '../identity';
|
||||
import session from 'express-session';
|
||||
import passport from 'passport';
|
||||
import { Minimatch } from 'minimatch';
|
||||
|
||||
type ProviderFactories = { [s: string]: AuthProviderFactory };
|
||||
|
||||
@@ -88,6 +89,8 @@ export async function createRouter({
|
||||
const providersConfig = config.getConfig('auth.providers');
|
||||
const configuredProviders = providersConfig.keys();
|
||||
|
||||
const isOriginAllowed = createOriginFilter(config);
|
||||
|
||||
for (const [providerId, providerFactory] of Object.entries(
|
||||
allProviderFactories,
|
||||
)) {
|
||||
@@ -96,7 +99,7 @@ export async function createRouter({
|
||||
try {
|
||||
const provider = providerFactory({
|
||||
providerId,
|
||||
globalConfig: { baseUrl: authUrl, appUrl },
|
||||
globalConfig: { baseUrl: authUrl, appUrl, isOriginAllowed },
|
||||
config: providersConfig.getConfig(providerId),
|
||||
logger,
|
||||
tokenIssuer,
|
||||
@@ -158,3 +161,22 @@ export async function createRouter({
|
||||
|
||||
return router;
|
||||
}
|
||||
|
||||
export function createOriginFilter(
|
||||
config: Config,
|
||||
): (origin: string) => boolean {
|
||||
const allowedOrigins = config.getOptionalStringArray(
|
||||
'auth.experimentalExtraAllowedOrigins',
|
||||
);
|
||||
if (!allowedOrigins || allowedOrigins.length === 0) {
|
||||
return () => false;
|
||||
}
|
||||
|
||||
const allowedOriginPatterns = allowedOrigins.map(
|
||||
pattern => new Minimatch(pattern, { nocase: true, noglobstar: true }),
|
||||
);
|
||||
|
||||
return origin => {
|
||||
return allowedOriginPatterns.some(pattern => pattern.match(origin));
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user