feat: Provide clear error message for unknown clients

* feat: Provide clear error message for unknown clients

* docs: Rephrase error message.

* docs: Update error message to reference local storage

---------

Co-authored-by: Ruben Verborgh <ruben@verborgh.org>
This commit is contained in:
Joachim Van Herwegen
2023-02-10 10:13:53 +01:00
committed by GitHub
parent 948dad7bcd
commit c332412074
3 changed files with 73 additions and 0 deletions

View File

@@ -19,6 +19,7 @@ import type { ResponseWriter } from '../../http/output/ResponseWriter';
import { BasicRepresentation } from '../../http/representation/BasicRepresentation';
import { getLoggerFor } from '../../logging/LogUtil';
import type { KeyValueStorage } from '../../storage/keyvalue/KeyValueStorage';
import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError';
import { InternalServerError } from '../../util/errors/InternalServerError';
import { RedirectHttpError } from '../../util/errors/RedirectHttpError';
import { guardStream } from '../../util/GuardedStream';
@@ -398,6 +399,22 @@ export class IdentityProviderFactory implements ProviderFactory {
}
}
// A client not being found is quite often the result of cookies being stored by the authn client,
// so we want to provide a more detailed error message explaining what to do.
if (oidcError.error_description === 'client is invalid' && oidcError.error_detail === 'client not found') {
const unknownClientError = new BadRequestHttpError(
'Unknown client, you might need to clear the local storage on the client.', {
errorCode: 'E0003',
details: {
client_id: ctx.request.query.client_id,
redirect_uri: ctx.request.query.redirect_uri,
},
},
);
unknownClientError.stack = error.stack;
error = unknownClientError;
}
const result = await this.errorHandler.handleSafe({ error, request: guardStream(ctx.req) });
await this.responseWriter.handleSafe({ response: ctx.res, result });
};

View File

@@ -0,0 +1,26 @@
# Authenticating with unknown client
You are trying to log in to an application,
but we can't proceed
because the app is using invalid settings.
To force the app to send us the right details,
delete the local storage in your browser for the site that sent you here.
Based on the data the app sent us,
this is probably `{{ redirect_uri }}`.
## Detailed error information
We received a request from a client with ID `{{ client_id }}`,
but this client is not registered with the server.
Probably,
this client was registered with the server in the past,
but it is no longer recognized
because some internal server data was removed.
Your data is still safe;
we just don't recognize the app's previous authentication anymore.
Because your browser still has the old authentication settings stored,
it tries to use them instead of setting up new ones.
By clearing those settings,
the application should automatically create a new client,
allowing you to log in again.

View File

@@ -59,6 +59,10 @@ describe('An IdentityProviderFactory', (): void => {
res: {},
request: {
href: 'http://example.com/idp/',
query: {
client_id: 'CLIENT_ID',
redirect_uri: 'REDIRECT_URI',
},
},
accepts: jest.fn().mockReturnValue('type'),
} as any;
@@ -235,6 +239,32 @@ describe('An IdentityProviderFactory', (): void => {
expect(error.stack).toContain('Error: bad data - more info - more details');
});
it('throws a specific error for unknown clients.', async(): Promise<void> => {
const provider = await factory.getProvider() as any;
const { config } = provider as { config: Configuration };
const error = new Error('invalid_client') as errors.OIDCProviderError;
error.error_description = 'client is invalid';
error.error_detail = 'client not found';
await expect((config.renderError as any)(ctx, {}, error)).resolves.toBeUndefined();
expect(errorHandler.handleSafe).toHaveBeenCalledTimes(1);
expect(errorHandler.handleSafe)
.toHaveBeenLastCalledWith({ error: expect.objectContaining({
statusCode: 400,
name: 'BadRequestHttpError',
message: 'Unknown client, you might need to clear the local storage on the client.',
errorCode: 'E0003',
details: {
client_id: 'CLIENT_ID',
redirect_uri: 'REDIRECT_URI',
},
}),
request: ctx.req });
expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1);
expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ response: ctx.res, result: { statusCode: 500 }});
});
it('adds middleware to make the OIDC provider think the request wants HTML.', async(): Promise<void> => {
const provider = await factory.getProvider() as any;
const use = provider.use as jest.Mock<ReturnType<Koa['use']>, Parameters<Koa['use']>>;