Custom error status on AuthenticationStrategy

[macabeus]

Is your feature request related to a problem? Please describe.
I'm developing a custom AuthenticationStrategy. It's working fine on the optimistic case.
But on my case I need to return custom errors message when a login fail, such as when the account is blocked.

But since the method authenticate could return only two types (User on success case, and false on fail case), I don't have a way to send a detailed error message to frontend display.

Describe the solution you'd like
We could change the authenticate to return something like that:

Promise<{
  success: true
  user: User
} |
{
  success: false
  error: string
}>

It's pretty flexible and type safe.

Describe alternatives you've considered
Another way, in order to not have break change, is changing the return type to Promise<User | false | string> - but it could be confused about what string means.

[michaelbromley]

Thanks for the suggestion - I agree and this should be quite straightforward to add.

Another way, in order to not have break change, is changing the return type to Promise<User | false | string> - but it could be confused about what string means.

I think this is ok, since it will be documented. Also, allowing a false return value means you are not forced to think up some error message if you don't need one.

[macabeus]

I agree and this should be quite straightforward to add.

Nice. I could try to add it, so.

[michaelbromley]

Yes, that would be helpful!

A few pointers:

I'd add a authenticationError: String! field to:

vendure/packages/core/src/api/schema/common/common-types.graphql

Lines 201 to 204 in db1893a

	type InvalidCredentialsError implements ErrorResult {
	errorCode: ErrorCode!
	message: String!
	}

Then if you run the codegen script it will update the generated class in this file and the constructor will now expect an argument for the authenticationError value.

Then in the AuthService you'd add a typeof user === 'string' check here and pass the value to the error constructor:

vendure/packages/core/src/service/services/auth.service.ts

Lines 57 to 60 in db1893a

	const user = await authenticationStrategy.authenticate(ctx, authenticationData);
	if (!user) {
	return new InvalidCredentialsError();
	}

Also adding an e2e test to https://github.com/vendure-ecommerce/vendure/blob/master/packages/core/e2e/authentication-strategy.e2e-spec.ts would be great.

[macabeus]

@michaelbromley I'm having this error on the types generated form GraphQL schema. Do you know how could I solve that? I can't understand very well why it's happening.

I tested manually and the new return worked fine. I started to have this error when I was pushing a wip commit and the pre-hook push ran some commands.

[michaelbromley]

Oh this is a bit of a pain, but because some types are shared by both the Admin and Shop APIs, they get generated in both the generated-types.ts file (Admin API) and generated-shop-types.ts file (Shop API). Then when you try to assign one to another TS complains. It can be quite tricky to solve (you'll see some cases in the BaseAuthResolver where I needed to import both types and make a union of them). If you can't figure it out, throw in an as any and add a TODO: correct typings and I'll take a look. Don't let it hold up the rest of the work.