fix: Apply OpenID role-sync fallback for present-but-empty claims

Both role-sync call sites skipped on a falsy `openIdRoleValues`, treating an
empty claim string ('') the same as a missing claim and returning before
`selectOpenIdRole` could apply the configured fallback role. An IdP emitting
an empty roles claim for a user with no mapped groups left the stale local
role in place instead of the authoritative fallback.

Skip only when the helper returns `undefined` (missing/invalid), letting an
empty string flow through to fallback selection — consistent with how an
empty array is already handled. Adds regression coverage on both the OpenID
strategy and the remote-agent API auth paths.
This commit is contained in:
Danny Avila 2026-05-30 11:23:56 -04:00
parent fc8ef5f662
commit a7821e2080
4 changed files with 36 additions and 2 deletions

View file

@ -510,7 +510,7 @@ async function applyOpenIdRoleSync({
decodeToken: jwtDecode,
resolveGroupOverage,
});
if (!openIdRoleValues) {
if (openIdRoleValues === undefined) {
logger.warn(
`[openidStrategy] OpenID role sync skipped; claim '${options.claim}' was not found, invalid, or unresolved`,
);

View file

@ -1652,6 +1652,24 @@ describe('setupOpenId', () => {
expect(user.role).toBe('USER');
});
it('uses fallback when the role claim is present but empty', async () => {
// The required-role gate reads the same `roles` claim this test empties, so
// disable it to model an IdP that authenticates the user yet emits no roles.
delete process.env.OPENID_REQUIRED_ROLE;
jwtDecode.mockReturnValue({
roles: '',
permissions: ['not-admin'],
});
const { user } = await validate(tokenset);
expect(user.role).toBe('USER');
expect(updateUser).toHaveBeenCalledWith(
'newUserId',
expect.objectContaining({ role: 'USER' }),
);
});
it('rejects login when configured sync roles do not exist', async () => {
findRolesByNames.mockImplementation(async (roleNames) =>
roleNames

View file

@ -1500,6 +1500,22 @@ describe('createRemoteAgentAuth', () => {
expect(req.user).toMatchObject({ role: 'USER' });
});
it('applies fallback when the role claim is present but empty', async () => {
enableApiRoleSync();
setupOidcMocks({
sub: 'sub123',
email: 'agent@test.com',
roles: '',
});
const deps = makeDeps();
const req = makeReq({ authorization: `Bearer ${FAKE_TOKEN}` });
await createRemoteAgentAuth(deps)(req as Request, makeRes().res, mockNext);
expect(deps.updateUser).toHaveBeenCalledWith('uid123', { role: 'USER' });
expect(req.user).toMatchObject({ role: 'USER' });
});
it('preserves an existing ADMIN role because generic role sync cannot manage admin', async () => {
enableApiRoleSync();
setupOidcMocks({

View file

@ -480,7 +480,7 @@ async function selectOpenIdRoleForOpenIdSync(
accessClaims: payload,
decodeToken: () => payload,
});
if (!openIdRoleValues) {
if (openIdRoleValues === undefined) {
logger.warn(
`[remoteAgentAuth] OpenID role sync skipped; claim '${options.claim}' was not found or invalid`,
);