Merge pull request #26969 from backstage/rugvip/harden

Update docs to be more strict around sign-in resolvers + remove insecure sign-in resolver
This commit is contained in:
Patrik Oldsberg
2024-10-08 11:53:37 +02:00
committed by GitHub
48 changed files with 186 additions and 276 deletions
+4 -1
View File
@@ -31,7 +31,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'usernameMatchingUserEntityName' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -41,7 +41,10 @@ export interface Config {
region: string;
signIn?: {
resolvers: Array<
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -29,7 +29,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'userIdMatchingUserEntityAnnotation' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
@@ -31,7 +31,10 @@ export interface Config {
authorizationCookieName?: string;
signIn?: {
resolvers: Array<
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -36,7 +36,10 @@ export interface Config {
resolvers: Array<
| { resolver: 'emailMatchingUserEntityAnnotation' }
| { resolver: 'idMatchingUserEntityAnnotation' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -31,7 +31,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'usernameMatchingUserEntityName' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -31,7 +31,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'usernameMatchingUserEntityName' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -30,7 +30,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'emailMatchingUserEntityAnnotation' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -33,7 +33,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'emailMatchingUserEntityAnnotation' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -35,7 +35,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'usernameMatchingUserEntityName' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -33,7 +33,10 @@ export interface Config {
prompt?: string;
signIn?: {
resolvers: Array<
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
@@ -39,7 +39,10 @@ export type OidcAuthResult = {
export namespace oidcSignInResolvers {
const emailLocalPartMatchingUserEntityName: SignInResolverFactory<
unknown,
unknown
| {
allowedDomains?: string[] | undefined;
}
| undefined
>;
const emailMatchingUserEntityProfileEmail: SignInResolverFactory<
unknown,
+4 -1
View File
@@ -33,7 +33,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'emailMatchingUserEntityAnnotation' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
+4 -1
View File
@@ -30,7 +30,10 @@ export interface Config {
signIn?: {
resolvers: Array<
| { resolver: 'usernameMatchingUserEntityName' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
@@ -27,8 +27,10 @@ export interface Config {
additionalScopes?: string | string[];
signIn?: {
resolvers: Array<
| { resolver: 'profileEmailMatchingUserEntityEmail' }
| { resolver: 'emailLocalPartMatchingUserEntityName' }
| {
resolver: 'emailLocalPartMatchingUserEntityName';
allowedDomains?: string[];
}
| { resolver: 'emailMatchingUserEntityProfileEmail' }
>;
};
@@ -5,10 +5,8 @@
```ts
import { BackendFeature } from '@backstage/backend-plugin-api';
import { OAuthAuthenticator } from '@backstage/plugin-auth-node';
import { OAuthAuthenticatorResult } from '@backstage/plugin-auth-node';
import { PassportOAuthAuthenticatorHelper } from '@backstage/plugin-auth-node';
import { PassportProfile } from '@backstage/plugin-auth-node';
import { SignInResolverFactory } from '@backstage/plugin-auth-node';
import { Strategy } from 'passport-oauth2';
// @public
@@ -31,14 +29,6 @@ export interface VMwareCloudAuthenticatorContext {
providerStrategy: Strategy;
}
// @public
export namespace vmwareCloudSignInResolvers {
const profileEmailMatchingUserEntityEmail: SignInResolverFactory<
OAuthAuthenticatorResult<PassportProfile>,
unknown
>;
}
// @public (undocumented)
export type VMwarePassportProfile = PassportProfile & {
organizationId?: string;
@@ -26,4 +26,3 @@ export {
type VMwarePassportProfile,
} from './authenticator';
export { authModuleVmwareCloudProvider as default } from './module';
export { vmwareCloudSignInResolvers } from './resolvers';
@@ -21,7 +21,6 @@ import {
} from '@backstage/plugin-auth-node';
import { vmwareCloudAuthenticator } from './authenticator';
import { vmwareCloudSignInResolvers } from './resolvers';
/**
* VMware Cloud Provider backend module for the auth plugin
@@ -40,7 +39,6 @@ export const authModuleVmwareCloudProvider = createBackendModule({
factory: createOAuthProviderFactory({
authenticator: vmwareCloudAuthenticator,
signInResolverFactories: {
...vmwareCloudSignInResolvers,
...commonSignInResolvers,
},
}),
@@ -1,90 +0,0 @@
/*
* Copyright 2023 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 { NotFoundError } from '@backstage/errors';
import {
AuthResolverContext,
OAuthAuthenticatorResult,
PassportProfile,
SignInInfo,
SignInResolver,
} from '@backstage/plugin-auth-node';
import { vmwareCloudSignInResolvers } from './resolvers';
describe('vmwareCloudResolver', () => {
let resolverContext: jest.Mocked<AuthResolverContext>;
let signInInfo: SignInInfo<OAuthAuthenticatorResult<PassportProfile>>;
let signInResolver: SignInResolver<OAuthAuthenticatorResult<PassportProfile>>;
beforeEach(() => {
resolverContext = {
issueToken: jest.fn().mockResolvedValue({
token: 'defaultBackstageToken',
}),
findCatalogUser: jest.fn(),
signInWithCatalogUser: jest.fn().mockResolvedValue({
token: 'backstageToken',
}),
};
signInInfo = {
result: {} as any, // Resolver doesn't care about the result object
profile: {
displayName: 'TestName',
email: 'user@example.com',
},
};
signInResolver =
vmwareCloudSignInResolvers.profileEmailMatchingUserEntityEmail();
});
it('looks up backstage identity by email', async () => {
const backstageIdentity = await signInResolver(signInInfo, resolverContext);
expect(backstageIdentity.token).toBe('backstageToken');
expect(resolverContext.signInWithCatalogUser).toHaveBeenCalledWith({
filter: {
'spec.profile.email': 'user@example.com',
},
});
});
it('returns "fake" backstage identity when no entity matches', async () => {
resolverContext.signInWithCatalogUser.mockRejectedValue(
new NotFoundError('User not found'),
);
const backstageIdentity = await signInResolver(signInInfo, resolverContext);
expect(backstageIdentity.token).toBe('defaultBackstageToken');
expect(resolverContext.issueToken).toHaveBeenCalledWith({
claims: {
sub: 'user:default/user@example.com',
ent: ['user:default/user@example.com'],
},
});
});
it('fails when resolver context throws other error', () => {
const error = new Error('bizarre');
resolverContext.signInWithCatalogUser.mockRejectedValue(error);
return expect(signInResolver(signInInfo, resolverContext)).rejects.toThrow(
error,
);
});
});
@@ -1,75 +0,0 @@
/*
* Copyright 2023 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 { stringifyEntityRef } from '@backstage/catalog-model';
import {
createSignInResolverFactory,
OAuthAuthenticatorResult,
PassportProfile,
SignInInfo,
} from '@backstage/plugin-auth-node';
/**
* Available sign-in resolvers for the VMware Cloud auth provider.
*
* @public
*/
export namespace vmwareCloudSignInResolvers {
/**
* Looks up the user by matching their profile email to the entity's profile email.
* If that fails, sign in the user without associating with a catalog user.
*/
export const profileEmailMatchingUserEntityEmail =
createSignInResolverFactory({
create() {
return async (
info: SignInInfo<OAuthAuthenticatorResult<PassportProfile>>,
ctx,
) => {
const email = info.profile.email;
if (!email) {
throw new Error(
'VMware login failed, user profile does not contain an email',
);
}
const userEntityRef = stringifyEntityRef({
kind: 'User',
name: email,
});
try {
// we await here so that signInWithCatalogUser throws in the current `try`
return await ctx.signInWithCatalogUser({
filter: {
'spec.profile.email': email,
},
});
} catch (e) {
if (e.name !== 'NotFoundError') {
throw e;
}
return ctx.issueToken({
claims: {
sub: userEntityRef,
ent: [userEntityRef],
},
});
}
};
},
});
}
+2 -1
View File
@@ -53,7 +53,8 @@
"passport": "^0.7.0",
"winston": "^3.2.1",
"zod": "^3.22.4",
"zod-to-json-schema": "^3.21.4"
"zod-to-json-schema": "^3.21.4",
"zod-validation-error": "^3.4.0"
},
"devDependencies": {
"@backstage/backend-test-utils": "workspace:^",
+7 -10
View File
@@ -148,7 +148,10 @@ export namespace commonSignInResolvers {
>;
const emailLocalPartMatchingUserEntityName: SignInResolverFactory<
unknown,
unknown
| {
allowedDomains?: string[] | undefined;
}
| undefined
>;
}
@@ -178,10 +181,7 @@ export function createOAuthProviderFactory<TProfile>(options: {
profileTransform?: ProfileTransform<OAuthAuthenticatorResult<TProfile>>;
signInResolver?: SignInResolver<OAuthAuthenticatorResult<TProfile>>;
signInResolverFactories?: {
[name in string]: SignInResolverFactory<
OAuthAuthenticatorResult<TProfile>,
unknown
>;
[name in string]: SignInResolverFactory;
};
}): AuthProviderFactory;
@@ -200,10 +200,7 @@ export function createProxyAuthProviderFactory<TResult>(options: {
authenticator: ProxyAuthenticator<unknown, TResult, unknown>;
profileTransform?: ProfileTransform<TResult>;
signInResolver?: SignInResolver<TResult>;
signInResolverFactories?: Record<
string,
SignInResolverFactory<TResult, unknown>
>;
signInResolverFactories?: Record<string, SignInResolverFactory>;
}): AuthProviderFactory;
// @public (undocumented)
@@ -648,7 +645,7 @@ export type SignInResolver<TAuthResult> = (
) => Promise<BackstageSignInResult>;
// @public (undocumented)
export interface SignInResolverFactory<TAuthResult, TOptions> {
export interface SignInResolverFactory<TAuthResult = any, TOptions = any> {
// (undocumented)
(
...options: undefined extends TOptions
@@ -34,10 +34,7 @@ export function createOAuthProviderFactory<TProfile>(options: {
profileTransform?: ProfileTransform<OAuthAuthenticatorResult<TProfile>>;
signInResolver?: SignInResolver<OAuthAuthenticatorResult<TProfile>>;
signInResolverFactories?: {
[name in string]: SignInResolverFactory<
OAuthAuthenticatorResult<TProfile>,
unknown
>;
[name in string]: SignInResolverFactory;
};
}): AuthProviderFactory {
return ctx => {
@@ -31,10 +31,7 @@ export function createProxyAuthProviderFactory<TResult>(options: {
authenticator: ProxyAuthenticator<unknown, TResult, unknown>;
profileTransform?: ProfileTransform<TResult>;
signInResolver?: SignInResolver<TResult>;
signInResolverFactories?: Record<
string,
SignInResolverFactory<TResult, unknown>
>;
signInResolverFactories?: Record<string, SignInResolverFactory>;
}): AuthProviderFactory {
return ctx => {
const signInResolver =
@@ -14,7 +14,9 @@
* limitations under the License.
*/
import { z } from 'zod';
import { createSignInResolverFactory } from './createSignInResolverFactory';
import { NotAllowedError } from '@backstage/errors';
// This splits an email "joe+work@acme.com" into ["joe", "+work", "@acme.com"]
// so that we can remove the plus addressing. May output a shorter array:
@@ -77,7 +79,13 @@ export namespace commonSignInResolvers {
*/
export const emailLocalPartMatchingUserEntityName =
createSignInResolverFactory({
create() {
optionsSchema: z
.object({
allowedDomains: z.array(z.string()).optional(),
})
.optional(),
create(options = {}) {
const { allowedDomains } = options;
return async (info, ctx) => {
const { profile } = info;
@@ -87,6 +95,13 @@ export namespace commonSignInResolvers {
);
}
const [localPart] = profile.email.split('@');
const domain = profile.email.slice(localPart.length + 1);
if (allowedDomains && !allowedDomains.includes(domain)) {
throw new NotAllowedError(
'Sign-in user email is not from an allowed domain',
);
}
return ctx.signInWithCatalogUser({
entityRef: { name: localPart },
@@ -18,10 +18,11 @@ import { ZodSchema, ZodTypeDef } from 'zod';
import { SignInResolver } from '../types';
import zodToJsonSchema from 'zod-to-json-schema';
import { JsonObject } from '@backstage/types';
import { fromError } from 'zod-validation-error';
import { InputError } from '@backstage/errors';
/** @public */
export interface SignInResolverFactory<TAuthResult, TOptions> {
export interface SignInResolverFactory<TAuthResult = any, TOptions = any> {
(
...options: undefined extends TOptions
? [options?: TOptions]
@@ -66,7 +67,14 @@ export function createSignInResolverFactory<
? [options?: TOptionsInput]
: [options: TOptionsInput]
) => {
const parsedOptions = optionsSchema.parse(resolverOptions);
let parsedOptions;
try {
parsedOptions = optionsSchema.parse(resolverOptions);
} catch (error) {
throw new InputError(
`Invalid sign-in resolver options, ${fromError(error)}`,
);
}
return options.create(parsedOptions);
};