diff --git a/.changeset/few-mice-buy.md b/.changeset/few-mice-buy.md new file mode 100644 index 000000000..a813aaa2b --- /dev/null +++ b/.changeset/few-mice-buy.md @@ -0,0 +1,5 @@ +--- +'hive': minor +--- + +Adds ability to select a default role for new OIDC users diff --git a/cypress/e2e/app.cy.ts b/cypress/e2e/app.cy.ts index dcee223f0..39f945abd 100644 --- a/cypress/e2e/app.cy.ts +++ b/cypress/e2e/app.cy.ts @@ -119,6 +119,36 @@ describe('oidc', () => { }); }); + it('default member role for first time oidc login', () => { + const organizationAdminUser = getUserData(); + cy.visit('/'); + cy.signup(organizationAdminUser); + + const slug = generateRandomSlug(); + cy.createOIDCIntegration(slug); + + // Pick Admin role as the default role + cy.get('[data-cy="role-selector-trigger"]').click(); + cy.contains('[data-cy="role-selector-item"]', 'Admin').click(); + cy.visit('/logout'); + + // First time login + cy.clearAllCookies(); + cy.clearAllLocalStorage(); + cy.clearAllSessionStorage(); + cy.get('a[href^="/auth/sso"]').click(); + cy.get('input[name="slug"]').type(slug); + cy.get('button[type="submit"]').click(); + // OIDC login + cy.get('input[id="Input_Username"]').type('test-user-2'); + cy.get('input[id="Input_Password"]').type('password'); + cy.get('button[value="login"]').click(); + + cy.get('[data-cy="organization-picker-current"]').contains(slug); + // Check if the user has the Admin role by checking if the Members tab is visible + cy.get(`a[href^="/${slug}/view/members"]`).should('exist'); + }); + it('oidc login for invalid url shows correct error message', () => { cy.clearAllCookies(); cy.clearAllLocalStorage(); diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 78e70d481..1b3541002 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -36,10 +36,6 @@ Cypress.Commands.add('createOIDCIntegration', (organizationSlug: string) => { cy.get('div[role="dialog"]').find('button[type="submit"]').last().click(); - cy.url().then(url => { - return new URL(url).pathname.split('/')[0]; - }); - return cy .get('div[role="dialog"]') .find('input[id="sign-in-uri"]') diff --git a/packages/migrations/src/actions/2025.01.13T10-08-00.default-role.ts b/packages/migrations/src/actions/2025.01.13T10-08-00.default-role.ts new file mode 100644 index 000000000..ed87fc89f --- /dev/null +++ b/packages/migrations/src/actions/2025.01.13T10-08-00.default-role.ts @@ -0,0 +1,25 @@ +import { type MigrationExecutor } from '../pg-migrator'; + +export default { + name: '2025.01.13T10-08-00.default-role.ts', + noTransaction: true, + // Adds a default role to OIDC integration and set index on "oidc_integrations"."default_role_id" + run: ({ sql }) => [ + { + name: 'Add a column', + query: sql` + ALTER TABLE "oidc_integrations" + ADD COLUMN IF NOT EXISTS "default_role_id" UUID REFERENCES organization_member_roles(id) + ON DELETE SET NULL; + `, + }, + { + name: 'Create an index', + query: sql` + CREATE INDEX CONCURRENTLY IF NOT EXISTS "oidc_integrations_default_role_id_idx" + ON "oidc_integrations"("default_role_id") + WHERE "default_role_id" is not null; + `, + }, + ], +} satisfies MigrationExecutor; diff --git a/packages/migrations/src/run-pg-migrations.ts b/packages/migrations/src/run-pg-migrations.ts index e662d2bd0..bcb8af6ed 100644 --- a/packages/migrations/src/run-pg-migrations.ts +++ b/packages/migrations/src/run-pg-migrations.ts @@ -154,5 +154,6 @@ export const runPGMigrations = async (args: { slonik: DatabasePool; runTo?: stri await import('./actions/2025.01.02T00-00-00.legacy-user-org-cleanup'), await import('./actions/2025.01.09T00-00-00.legacy-member-scopes'), await import('./actions/2025.01.10T00.00.00.breaking-changes-request-count'), + await import('./actions/2025.01.13T10-08-00.default-role'), ], }); diff --git a/packages/services/api/src/modules/oidc-integrations/module.graphql.ts b/packages/services/api/src/modules/oidc-integrations/module.graphql.ts index 931bf1d34..40cbee055 100644 --- a/packages/services/api/src/modules/oidc-integrations/module.graphql.ts +++ b/packages/services/api/src/modules/oidc-integrations/module.graphql.ts @@ -19,6 +19,7 @@ export default gql` authorizationEndpoint: String! organization: Organization! oidcUserAccessOnly: Boolean! + defaultMemberRole: MemberRole! } extend type Mutation { @@ -26,6 +27,9 @@ export default gql` updateOIDCIntegration(input: UpdateOIDCIntegrationInput!): UpdateOIDCIntegrationResult! deleteOIDCIntegration(input: DeleteOIDCIntegrationInput!): DeleteOIDCIntegrationResult! updateOIDCRestrictions(input: UpdateOIDCRestrictionsInput!): UpdateOIDCRestrictionsResult! + updateOIDCDefaultMemberRole( + input: UpdateOIDCDefaultMemberRoleInput! + ): UpdateOIDCDefaultMemberRoleResult! } type Subscription { @@ -149,4 +153,25 @@ export default gql` type UpdateOIDCRestrictionsError implements Error { message: String! } + + input UpdateOIDCDefaultMemberRoleInput { + oidcIntegrationId: ID! + defaultMemberRoleId: ID! + } + + """ + @oneOf + """ + type UpdateOIDCDefaultMemberRoleResult { + ok: UpdateOIDCDefaultMemberRoleOk + error: UpdateOIDCDefaultMemberRoleError + } + + type UpdateOIDCDefaultMemberRoleOk { + updatedOIDCIntegration: OIDCIntegration! + } + + type UpdateOIDCDefaultMemberRoleError implements Error { + message: String! + } `; diff --git a/packages/services/api/src/modules/oidc-integrations/providers/oidc-integrations.provider.ts b/packages/services/api/src/modules/oidc-integrations/providers/oidc-integrations.provider.ts index e83ba1689..e0533376e 100644 --- a/packages/services/api/src/modules/oidc-integrations/providers/oidc-integrations.provider.ts +++ b/packages/services/api/src/modules/oidc-integrations/providers/oidc-integrations.provider.ts @@ -363,6 +363,57 @@ export class OIDCIntegrationsProvider { } as const; } + async updateOIDCDefaultMemberRole(args: { oidcIntegrationId: string; roleId: string }) { + if (this.isEnabled() === false) { + return { + type: 'error', + message: 'OIDC integrations are disabled.', + } as const; + } + + const oidcIntegration = await this.storage.getOIDCIntegrationById({ + oidcIntegrationId: args.oidcIntegrationId, + }); + + if (oidcIntegration === null) { + return { + type: 'error', + message: 'Integration not found.', + } as const; + } + + const viewer = await this.session.getViewer(); + const [member, adminRole] = await Promise.all([ + this.storage.getOrganizationMember({ + organizationId: oidcIntegration.linkedOrganizationId, + userId: viewer.id, + }), + this.storage.getAdminOrganizationMemberRole({ + organizationId: oidcIntegration.linkedOrganizationId, + }), + ]); + + if (member?.role.id !== adminRole.id) { + return { + type: 'error', + message: 'You do not have permission to update the default member role.', + } as const; + } + + await this.session.assertPerformAction({ + organizationId: oidcIntegration.linkedOrganizationId, + action: 'oidc:modify', + params: { + organizationId: oidcIntegration.linkedOrganizationId, + }, + }); + + return { + type: 'ok', + oidcIntegration: await this.storage.updateOIDCDefaultMemberRole(args), + } as const; + } + async getOIDCIntegrationById(args: { oidcIntegrationId: string }) { if (this.isEnabled() === false) { return { diff --git a/packages/services/api/src/modules/oidc-integrations/resolvers/Mutation/updateOIDCDefaultMemberRole.ts b/packages/services/api/src/modules/oidc-integrations/resolvers/Mutation/updateOIDCDefaultMemberRole.ts new file mode 100644 index 000000000..d7f47a2cf --- /dev/null +++ b/packages/services/api/src/modules/oidc-integrations/resolvers/Mutation/updateOIDCDefaultMemberRole.ts @@ -0,0 +1,25 @@ +import { OIDCIntegrationsProvider } from '../../providers/oidc-integrations.provider'; +import type { MutationResolvers } from './../../../../__generated__/types'; + +export const updateOIDCDefaultMemberRole: NonNullable< + MutationResolvers['updateOIDCDefaultMemberRole'] +> = async (_, { input }, { injector }) => { + const result = await injector.get(OIDCIntegrationsProvider).updateOIDCDefaultMemberRole({ + roleId: input.defaultMemberRoleId, + oidcIntegrationId: input.oidcIntegrationId, + }); + + if (result.type === 'error') { + return { + error: { + message: result.message, + }, + }; + } + + return { + ok: { + updatedOIDCIntegration: result.oidcIntegration, + }, + }; +}; diff --git a/packages/services/api/src/modules/oidc-integrations/resolvers/OIDCIntegration.ts b/packages/services/api/src/modules/oidc-integrations/resolvers/OIDCIntegration.ts index cc4babd99..0febf8262 100644 --- a/packages/services/api/src/modules/oidc-integrations/resolvers/OIDCIntegration.ts +++ b/packages/services/api/src/modules/oidc-integrations/resolvers/OIDCIntegration.ts @@ -1,3 +1,4 @@ +import { OrganizationManager } from '../../organization/providers/organization-manager'; import { OIDCIntegrationsProvider } from '../providers/oidc-integrations.provider'; import type { OidcIntegrationResolvers } from './../../../__generated__/types'; @@ -9,4 +10,27 @@ export const OIDCIntegration: OidcIntegrationResolvers = { clientId: oidcIntegration => oidcIntegration.clientId, clientSecretPreview: (oidcIntegration, _, { injector }) => injector.get(OIDCIntegrationsProvider).getClientSecretPreview(oidcIntegration), + /** + * Fallbacks to Viewer if default member role is not set + */ + defaultMemberRole: async (oidcIntegration, _, { injector }) => { + if (!oidcIntegration.defaultMemberRoleId) { + return injector.get(OrganizationManager).getViewerMemberRole({ + organizationId: oidcIntegration.linkedOrganizationId, + }); + } + + const role = await injector.get(OrganizationManager).getMemberRole({ + organizationId: oidcIntegration.linkedOrganizationId, + roleId: oidcIntegration.defaultMemberRoleId, + }); + + if (!role) { + throw new Error( + `Default role not found (role_id=${oidcIntegration.defaultMemberRoleId}, organization=${oidcIntegration.linkedOrganizationId})`, + ); + } + + return role; + }, }; diff --git a/packages/services/api/src/modules/organization/providers/organization-manager.ts b/packages/services/api/src/modules/organization/providers/organization-manager.ts index 2c6ad79de..275aa298c 100644 --- a/packages/services/api/src/modules/organization/providers/organization-manager.ts +++ b/packages/services/api/src/modules/organization/providers/organization-manager.ts @@ -1419,6 +1419,20 @@ export class OrganizationManager { }); } + async getViewerMemberRole(selector: { organizationId: string }) { + await this.session.assertPerformAction({ + action: 'member:describe', + organizationId: selector.organizationId, + params: { + organizationId: selector.organizationId, + }, + }); + + return this.storage.getViewerOrganizationMemberRole({ + organizationId: selector.organizationId, + }); + } + async canDeleteRole( role: OrganizationMemberRole, currentUserScopes: readonly ( diff --git a/packages/services/api/src/modules/shared/providers/storage.ts b/packages/services/api/src/modules/shared/providers/storage.ts index 8c4c427e2..ef105a5d5 100644 --- a/packages/services/api/src/modules/shared/providers/storage.ts +++ b/packages/services/api/src/modules/shared/providers/storage.ts @@ -638,6 +638,11 @@ export interface Storage { oidcUserAccessOnly: boolean; }): Promise; + updateOIDCDefaultMemberRole(_: { + oidcIntegrationId: string; + roleId: string; + }): Promise; + createCDNAccessToken(_: { id: string; targetId: string; diff --git a/packages/services/api/src/shared/entities.ts b/packages/services/api/src/shared/entities.ts index 34307797a..f61f85abd 100644 --- a/packages/services/api/src/shared/entities.ts +++ b/packages/services/api/src/shared/entities.ts @@ -219,6 +219,7 @@ export interface OIDCIntegration { userinfoEndpoint: string; authorizationEndpoint: string; oidcUserAccessOnly: boolean; + defaultMemberRoleId: string | null; } export interface CDNAccessToken { diff --git a/packages/services/storage/src/db/types.ts b/packages/services/storage/src/db/types.ts index b08109c5a..deb2a52f9 100644 --- a/packages/services/storage/src/db/types.ts +++ b/packages/services/storage/src/db/types.ts @@ -157,6 +157,7 @@ export interface oidc_integrations { client_id: string; client_secret: string; created_at: Date; + default_role_id: string | null; id: string; linked_organization_id: string; oauth_api_url: string | null; diff --git a/packages/services/storage/src/index.ts b/packages/services/storage/src/index.ts index f343e0594..90d6ed5d5 100644 --- a/packages/services/storage/src/index.ts +++ b/packages/services/storage/src/index.ts @@ -497,17 +497,24 @@ export async function createStorage( return; } - const viewerRole = await shared.getOrganizationMemberRoleByName( - { organizationId: linkedOrganizationId, roleName: 'Viewer' }, - connection, - ); - // TODO: turn it into a default role and let the admin choose the default role + // Add user and assign default role (either Viewer or custom default role) await connection.query( sql`/* addOrganizationMemberViaOIDCIntegrationId */ INSERT INTO organization_member (organization_id, user_id, role_id) VALUES - (${linkedOrganizationId}, ${args.userId}, ${viewerRole.id}) + ( + ${linkedOrganizationId}, + ${args.userId}, + ( + COALESCE( + (SELECT default_role_id FROM oidc_integrations + WHERE id = ${args.oidcIntegrationId}), + (SELECT id FROM organization_member_roles + WHERE organization_id = ${linkedOrganizationId} AND name = 'Viewer') + ) + ) + ) ON CONFLICT DO NOTHING RETURNING * `, @@ -605,7 +612,6 @@ export async function createStorage( { oidcIntegrationId: oidcIntegration.id, userId: internalUser.id, - // TODO: pass a default role here }, t, ); @@ -3051,6 +3057,7 @@ export async function createStorage( , "userinfo_endpoint" , "authorization_endpoint" , "oidc_user_access_only" + , "default_role_id" FROM "oidc_integrations" WHERE @@ -3077,6 +3084,7 @@ export async function createStorage( , "userinfo_endpoint" , "authorization_endpoint" , "oidc_user_access_only" + , "default_role_id" FROM "oidc_integrations" WHERE @@ -3139,6 +3147,7 @@ export async function createStorage( , "userinfo_endpoint" , "authorization_endpoint" , "oidc_user_access_only" + , "default_role_id" `); return { @@ -3193,6 +3202,7 @@ export async function createStorage( , "userinfo_endpoint" , "authorization_endpoint" , "oidc_user_access_only" + , "default_role_id" `); return decodeOktaIntegrationRecord(result); @@ -3215,11 +3225,51 @@ export async function createStorage( , "userinfo_endpoint" , "authorization_endpoint" , "oidc_user_access_only" + , "default_role_id" `); return decodeOktaIntegrationRecord(result); }, + async updateOIDCDefaultMemberRole(args) { + return tracedTransaction('updateOIDCDefaultMemberRole', pool, async trx => { + // Make sure the role exists and is associated with the organization + const roleId = await trx.oneFirst(sql`/* checkRoleExists */ + SELECT id FROM "organization_member_roles" + WHERE + "id" = ${args.roleId} AND + "organization_id" = ( + SELECT "linked_organization_id" FROM "oidc_integrations" WHERE "id" = ${args.oidcIntegrationId} + ) + `); + + if (!roleId) { + throw new Error('Role does not exist'); + } + + const result = await pool.one(sql`/* updateOIDCDefaultMemberRole */ + UPDATE "oidc_integrations" + SET + "default_role_id" = ${roleId} + WHERE + "id" = ${args.oidcIntegrationId} + RETURNING + "id" + , "linked_organization_id" + , "client_id" + , "client_secret" + , "oauth_api_url" + , "token_endpoint" + , "userinfo_endpoint" + , "authorization_endpoint" + , "oidc_user_access_only" + , "default_role_id" + `); + + return decodeOktaIntegrationRecord(result); + }); + }, + async deleteOIDCIntegration(args) { await pool.query(sql`/* deleteOIDCIntegration */ DELETE FROM "oidc_integrations" @@ -4575,6 +4625,7 @@ const OktaIntegrationBaseModel = zod.object({ client_id: zod.string(), client_secret: zod.string(), oidc_user_access_only: zod.boolean(), + default_role_id: zod.string().nullable(), }); const OktaIntegrationLegacyModel = zod.intersection( @@ -4609,6 +4660,7 @@ const decodeOktaIntegrationRecord = (result: unknown): OIDCIntegration => { userinfoEndpoint: `${rawRecord.oauth_api_url}/userinfo`, authorizationEndpoint: `${rawRecord.oauth_api_url}/authorize`, oidcUserAccessOnly: rawRecord.oidc_user_access_only, + defaultMemberRoleId: rawRecord.default_role_id, }; } @@ -4621,6 +4673,7 @@ const decodeOktaIntegrationRecord = (result: unknown): OIDCIntegration => { userinfoEndpoint: rawRecord.userinfo_endpoint, authorizationEndpoint: rawRecord.authorization_endpoint, oidcUserAccessOnly: rawRecord.oidc_user_access_only, + defaultMemberRoleId: rawRecord.default_role_id, }; }; diff --git a/packages/web/app/src/components/organization/members/common.tsx b/packages/web/app/src/components/organization/members/common.tsx index 2084c3517..9ce146ac5 100644 --- a/packages/web/app/src/components/organization/members/common.tsx +++ b/packages/web/app/src/components/organization/members/common.tsx @@ -29,6 +29,7 @@ export function RoleSelector(props: { reason?: string; }; disabled?: boolean; + searchPlaceholder?: string; onSelect(role: Role): void | Promise; /** * It's only needed for the migration flow, where we need to be able to select no role. @@ -46,7 +47,8 @@ export function RoleSelector(props: { - - - - - ) : ( + if (oidcIntegration == null) { + return ( + + + + Manage OpenID Connect Integration + + You are trying to update an OpenID Connect integration for an organization that has no + integration. + + + + + + + + + ); + } + + if (!props.memberRoles) { + console.error('ManageOIDCIntegrationModal is missing member roles'); + return null; + } + + return ( + ); +} + +const OIDCDefaultRoleSelector_MemberRoleFragment = graphql(` + fragment OIDCDefaultRoleSelector_MemberRoleFragment on MemberRole { + id + name + description + } +`); + +const OIDCDefaultRoleSelector_UpdateMutation = graphql(` + mutation OIDCDefaultRoleSelector_UpdateMutation($input: UpdateOIDCDefaultMemberRoleInput!) { + updateOIDCDefaultMemberRole(input: $input) { + ok { + updatedOIDCIntegration { + id + defaultMemberRole { + ...OIDCDefaultRoleSelector_MemberRoleFragment + } + } + } + error { + message + } + } + } +`); + +function OIDCDefaultRoleSelector(props: { + oidcIntegrationId: string; + disabled: boolean; + defaultRole: FragmentType; + memberRoles: Array>; +}) { + const defaultRole = useFragment(OIDCDefaultRoleSelector_MemberRoleFragment, props.defaultRole); + const memberRoles = useFragment(OIDCDefaultRoleSelector_MemberRoleFragment, props.memberRoles); + const [_, mutate] = useMutation(OIDCDefaultRoleSelector_UpdateMutation); + const { toast } = useToast(); + + return ( + { + if (role.id === defaultRole.id) { + return; + } + + try { + const result = await mutate({ + input: { + oidcIntegrationId: props.oidcIntegrationId, + defaultMemberRoleId: role.id, + }, + }); + + if (result.data?.updateOIDCDefaultMemberRole.ok) { + toast({ + title: 'Default member role updated', + description: `${role.name} is now the default role for new OIDC members`, + }); + return; + } + + toast({ + title: 'Failed to update default member role', + description: + result.data?.updateOIDCDefaultMemberRole.error?.message ?? + result.error?.message ?? + 'Please try again later', + }); + } catch (error) { + toast({ + title: 'Failed to update default member role', + description: 'Please try again later', + variant: 'destructive', + }); + console.error(error); + } + }} + isRoleActive={_ => true} /> ); } @@ -594,6 +707,10 @@ const UpdateOIDCIntegration_OIDCIntegrationFragment = graphql(` clientId clientSecretPreview oidcUserAccessOnly + defaultMemberRole { + id + ...OIDCDefaultRoleSelector_MemberRoleFragment + } } `); @@ -648,6 +765,8 @@ function UpdateOIDCIntegrationForm(props: { close: () => void; isOpen: boolean; oidcIntegration: DocumentType; + isAdmin: boolean; + memberRoles: Array>; }): ReactElement { const [oidcUpdateMutation, oidcUpdateMutate] = useMutation( UpdateOIDCIntegrationForm_UpdateOIDCIntegrationMutation, @@ -771,8 +890,8 @@ function UpdateOIDCIntegrationForm(props: {
-
Restrictions
-
+
Other Options
+

OIDC-Only Access

@@ -790,6 +909,28 @@ function UpdateOIDCIntegrationForm(props: { disabled={oidcRestrictionsMutation.fetching} />
+
+
+

Default Member Role

+

+ This role is assigned to new members who sign in via OIDC.{' '} + + Only members with the Admin role can modify it. + +

+
+ +