From 3c613754f041d37e71ba5bb0945745075f6001ed Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Oct 2018 12:57:20 -0600 Subject: [PATCH] feat: added allowRegistration option --- .../app/handlers/api/tenant/auth/local.ts | 5 + .../passport/strategies/oidc/index.ts | 23 ++--- .../passport/strategies/oidc/oidc.spec.ts | 13 +-- .../passport/strategies/verifiers/sso.spec.ts | 9 +- .../passport/strategies/verifiers/sso.ts | 24 +++-- .../server/graph/tenant/schema/schema.graphql | 99 +++++++++++++++---- src/core/server/models/tenant.ts | 4 + src/core/server/services/tenant/index.ts | 1 + 8 files changed, 121 insertions(+), 57 deletions(-) diff --git a/src/core/server/app/handlers/api/tenant/auth/local.ts b/src/core/server/app/handlers/api/tenant/auth/local.ts index 181148b87..94f8fc7f9 100644 --- a/src/core/server/app/handlers/api/tenant/auth/local.ts +++ b/src/core/server/app/handlers/api/tenant/auth/local.ts @@ -51,6 +51,11 @@ export const signupHandler = (options: SignupOptions): RequestHandler => async ( return next(new Error("integration is disabled")); } + if (!tenant.auth.integrations.local.allowRegistration) { + // TODO: replace with better error. + return next(new Error("registration is disabled")); + } + // Get the fields from the body. Validate will throw an error if the body // does not conform to the specification. const { username, password, email }: SignupBody = validate( diff --git a/src/core/server/app/middleware/passport/strategies/oidc/index.ts b/src/core/server/app/middleware/passport/strategies/oidc/index.ts index b9ff305a7..1d792f5f1 100644 --- a/src/core/server/app/middleware/passport/strategies/oidc/index.ts +++ b/src/core/server/app/middleware/passport/strategies/oidc/index.ts @@ -136,13 +136,10 @@ export const OIDCIDTokenSchema = Joi.object() email: Joi.string(), email_verified: Joi.boolean().default(false), picture: Joi.string().default(undefined), + name: Joi.string().default(undefined), + nickname: Joi.string().default(undefined), }) - .optionalKeys(["picture", "email_verified"]); - -export const OIDCDisplayNameIDTokenSchema = OIDCIDTokenSchema.keys({ - name: Joi.string().default(undefined), - nickname: Joi.string().default(undefined), -}).optionalKeys(["name", "nickname"]); + .optionalKeys(["picture", "email_verified", "name", "nickname"]); export async function findOrCreateOIDCUser( db: Db, @@ -160,12 +157,7 @@ export async function findOrCreateOIDCUser( picture, name, nickname, - }: OIDCIDToken = validate( - integration.displayNameEnable - ? OIDCDisplayNameIDTokenSchema - : OIDCIDTokenSchema, - token - ); + }: OIDCIDToken = validate(OIDCIDTokenSchema, token); // Construct the profile that will be used to query for the user. const profile: OIDCProfile = { @@ -178,10 +170,13 @@ export async function findOrCreateOIDCUser( // Try to lookup user given their id provided in the `sub` claim. let user = await retrieveUserWithProfile(db, tenant.id, profile); if (!user) { + if (!integration.allowRegistration) { + // Registration is disabled, so we can't create the user user here. + return; + } + // FIXME: implement rules. - // Default the displayName. When it is disabled, Joi will strip the - // displayName fields from the token, so it will fallback to undefined. const displayName = nickname || name || undefined; // Create the new user, as one didn't exist before! diff --git a/src/core/server/app/middleware/passport/strategies/oidc/oidc.spec.ts b/src/core/server/app/middleware/passport/strategies/oidc/oidc.spec.ts index 141b145ba..b8afbffad 100644 --- a/src/core/server/app/middleware/passport/strategies/oidc/oidc.spec.ts +++ b/src/core/server/app/middleware/passport/strategies/oidc/oidc.spec.ts @@ -1,7 +1,4 @@ -import { - OIDCDisplayNameIDTokenSchema, - OIDCIDTokenSchema, -} from "talk-server/app/middleware/passport/strategies/oidc"; +import { OIDCIDTokenSchema } from "talk-server/app/middleware/passport/strategies/oidc"; import { validate } from "talk-server/app/request/body"; describe("OIDCIDTokenSchema", () => { @@ -42,9 +39,7 @@ describe("OIDCIDTokenSchema", () => { expect(validate(OIDCIDTokenSchema, token)).toEqual(token); }); -}); -describe("OIDCDisplayNameIDTokenSchema", () => { it("allows a valid payload", () => { const token = { sub: "sub", @@ -56,7 +51,7 @@ describe("OIDCDisplayNameIDTokenSchema", () => { nickname: "nickname", }; - expect(validate(OIDCDisplayNameIDTokenSchema, token)).toEqual(token); + expect(validate(OIDCIDTokenSchema, token)).toEqual(token); }); it("allows an empty name", () => { @@ -69,7 +64,7 @@ describe("OIDCDisplayNameIDTokenSchema", () => { nickname: "nickname", }; - expect(validate(OIDCDisplayNameIDTokenSchema, token)).toEqual(token); + expect(validate(OIDCIDTokenSchema, token)).toEqual(token); }); it("allows an empty nickname", () => { @@ -82,6 +77,6 @@ describe("OIDCDisplayNameIDTokenSchema", () => { name: "name", }; - expect(validate(OIDCDisplayNameIDTokenSchema, token)).toEqual(token); + expect(validate(OIDCIDTokenSchema, token)).toEqual(token); }); }); diff --git a/src/core/server/app/middleware/passport/strategies/verifiers/sso.spec.ts b/src/core/server/app/middleware/passport/strategies/verifiers/sso.spec.ts index 7f9816541..049fe45e9 100644 --- a/src/core/server/app/middleware/passport/strategies/verifiers/sso.spec.ts +++ b/src/core/server/app/middleware/passport/strategies/verifiers/sso.spec.ts @@ -1,6 +1,5 @@ import { isSSOToken, - SSODisplayNameUserProfileSchema, SSOUserProfileSchema, } from "talk-server/app/middleware/passport/strategies/verifiers/sso"; import { validate } from "talk-server/app/request/body"; @@ -44,9 +43,7 @@ describe("SSOUserProfileSchema", () => { expect(validate(SSOUserProfileSchema, profile)).toEqual(profile); }); -}); -describe("SSODisplayNameUserProfileSchema", () => { it("allows a valid payload", () => { const profile = { id: "id", @@ -56,7 +53,7 @@ describe("SSODisplayNameUserProfileSchema", () => { displayName: "displayName", }; - expect(validate(SSODisplayNameUserProfileSchema, profile)).toEqual(profile); + expect(validate(SSOUserProfileSchema, profile)).toEqual(profile); }); it("allows an empty avatar", () => { @@ -67,7 +64,7 @@ describe("SSODisplayNameUserProfileSchema", () => { displayName: "displayName", }; - expect(validate(SSODisplayNameUserProfileSchema, profile)).toEqual(profile); + expect(validate(SSOUserProfileSchema, profile)).toEqual(profile); }); it("allows an empty displayName", () => { @@ -78,6 +75,6 @@ describe("SSODisplayNameUserProfileSchema", () => { avatar: "avatar", }; - expect(validate(SSODisplayNameUserProfileSchema, profile)).toEqual(profile); + expect(validate(SSOUserProfileSchema, profile)).toEqual(profile); }); }); diff --git a/src/core/server/app/middleware/passport/strategies/verifiers/sso.ts b/src/core/server/app/middleware/passport/strategies/verifiers/sso.ts index 1ff9ff44f..60c606b53 100644 --- a/src/core/server/app/middleware/passport/strategies/verifiers/sso.ts +++ b/src/core/server/app/middleware/passport/strategies/verifiers/sso.ts @@ -3,7 +3,10 @@ import jwt from "jsonwebtoken"; import { Db } from "mongodb"; import { validate } from "talk-server/app/request/body"; -import { GQLUSER_ROLE } from "talk-server/graph/tenant/schema/__generated__/types"; +import { + GQLSSOAuthIntegration, + GQLUSER_ROLE, +} from "talk-server/graph/tenant/schema/__generated__/types"; import { Tenant } from "talk-server/models/tenant"; import { retrieveUserWithProfile, SSOProfile } from "talk-server/models/user"; import { upsert } from "talk-server/services/users"; @@ -30,16 +33,14 @@ export const SSOUserProfileSchema = Joi.object() email: Joi.string(), username: Joi.string(), avatar: Joi.string().default(undefined), + displayName: Joi.string().default(undefined), }) - .optionalKeys(["avatar"]); - -export const SSODisplayNameUserProfileSchema = SSOUserProfileSchema.keys({ - displayName: Joi.string().default(undefined), -}).optionalKeys(["displayName"]); + .optionalKeys(["avatar", "displayName"]); export async function findOrCreateSSOUser( db: Db, tenant: Tenant, + integration: GQLSSOAuthIntegration, token: SSOToken ) { if (!token.user) { @@ -49,9 +50,7 @@ export async function findOrCreateSSOUser( // Unpack/validate the token content. const { id, email, username, displayName, avatar }: SSOUserProfile = validate( - tenant.auth.integrations.sso!.displayNameEnable - ? SSODisplayNameUserProfileSchema - : SSOUserProfileSchema, + SSOUserProfileSchema, token.user ); @@ -63,6 +62,11 @@ export async function findOrCreateSSOUser( // Try to lookup user given their id provided in the `sub` claim. let user = await retrieveUserWithProfile(db, tenant.id, profile); if (!user) { + if (!integration.allowRegistration) { + // Registration is disabled, so we can't create the user user here. + return; + } + // FIXME: (wyattjoh) implement rules! Not all users should be able to create an account via this method. // Create the new user, as one didn't exist before! @@ -138,6 +142,6 @@ export class SSOVerifier { algorithms: ["HS256"], // TODO: (wyattjoh) investigate replacing algorithm. }); - return findOrCreateSSOUser(this.mongo, tenant, token); + return findOrCreateSSOUser(this.mongo, tenant, integration, token); } } diff --git a/src/core/server/graph/tenant/schema/schema.graphql b/src/core/server/graph/tenant/schema/schema.graphql index 197530ce5..e845393bd 100644 --- a/src/core/server/graph/tenant/schema/schema.graphql +++ b/src/core/server/graph/tenant/schema/schema.graphql @@ -264,6 +264,12 @@ type AuthenticationTargetFilter { type LocalAuthIntegration { enabled: Boolean! + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean! + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication @@ -284,6 +290,12 @@ embed to allow single sign on. type SSOAuthIntegration { enabled: Boolean! + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean! + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication @@ -300,12 +312,6 @@ type SSOAuthIntegration { keyGeneratedAt is the Time that the key was effective from. """ keyGeneratedAt: Time @auth(roles: [ADMIN]) - - """ - displayNameEnable when enabled, will allow Users to set and view their - displayName's. - """ - displayNameEnable: Boolean @auth(roles: [ADMIN]) } ########################## @@ -368,6 +374,12 @@ type OIDCAuthIntegration { """ enabled: Boolean! + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean! + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication @@ -430,12 +442,6 @@ type OIDCAuthIntegration { https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata """ issuer: String! @auth(roles: [ADMIN]) - - """ - displayNameEnable when enabled, will allow Users to set and view their - displayName's. - """ - displayNameEnable: Boolean @auth(roles: [ADMIN]) } ########################## @@ -445,6 +451,12 @@ type OIDCAuthIntegration { type GoogleAuthIntegration { enabled: Boolean! + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean! + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication @@ -463,6 +475,12 @@ type GoogleAuthIntegration { type FacebookAuthIntegration { enabled: Boolean! + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean! + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication @@ -474,6 +492,10 @@ type FacebookAuthIntegration { clientSecret: String @auth(roles: [ADMIN]) } +########################## +## Auth +########################## + type AuthIntegrations { local: LocalAuthIntegration! sso: SSOAuthIntegration! @@ -482,6 +504,17 @@ type AuthIntegrations { facebook: FacebookAuthIntegration! } +""" +AuthDisplayNameConfiguration allows configuration related to Display Names. +""" +type AuthDisplayNameConfiguration { + """ + enabled when true will allow the display name to be used by other + AuthIntegrations. + """ + enabled: Boolean! +} + """ Auth contains all the settings related to authentication and authorization. @@ -492,6 +525,12 @@ type Auth { authentication solutions. """ integrations: AuthIntegrations! + + """ + displayName contains configuration related to the use of Display Names across + AuthIntegrations. + """ + displayName: AuthDisplayNameConfiguration @auth(roles: [ADMIN]) } ################################################################################ @@ -1431,6 +1470,12 @@ input SettingsAuthenticationTargetFilterInput { input SettingsLocalAuthIntegrationInput { enabled: Boolean + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication @@ -1442,23 +1487,29 @@ input SettingsLocalAuthIntegrationInput { input SettingsSSOAuthIntegrationInput { enabled: Boolean + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication integration should be displayed in all targets. """ targetFilter: SettingsAuthenticationTargetFilterInput - - """ - displayNameEnable when enabled, will allow Users to set and view their - displayName's. - """ - displayNameEnable: Boolean } input SettingsGoogleAuthIntegrationInput { enabled: Boolean + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication @@ -1473,6 +1524,12 @@ input SettingsGoogleAuthIntegrationInput { input SettingsFacebookAuthIntegrationInput { enabled: Boolean + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication @@ -2072,6 +2129,12 @@ input UpdateOIDCAuthIntegrationConfigurationInput { """ enabled: Boolean + """ + allowRegistration when true will allow users that have not signed up + before with this authentication integration to sign up. + """ + allowRegistration: Boolean + """ targetFilter will restrict where the authentication integration should be displayed. If the value of targetFilter is null, then the authentication diff --git a/src/core/server/models/tenant.ts b/src/core/server/models/tenant.ts index 559cf7178..ad2cc9652 100644 --- a/src/core/server/models/tenant.ts +++ b/src/core/server/models/tenant.ts @@ -86,16 +86,20 @@ export async function createTenant(mongo: Db, input: CreateTenantInput) { integrations: { local: { enabled: true, + allowRegistration: true, }, sso: { enabled: false, + allowRegistration: false, }, oidc: [], google: { enabled: false, + allowRegistration: false, }, facebook: { enabled: false, + allowRegistration: false, }, }, }, diff --git a/src/core/server/services/tenant/index.ts b/src/core/server/services/tenant/index.ts index c484f3c83..0fa525616 100644 --- a/src/core/server/services/tenant/index.ts +++ b/src/core/server/services/tenant/index.ts @@ -136,6 +136,7 @@ export async function createOIDCAuthIntegration( // Create the integration. By default, the integration is disabled. const result = await createTenantOIDCAuthIntegration(mongo, tenant.id, { enabled: false, + allowRegistration: false, ...input, }); if (!result.wasCreated || !result.tenant) {