Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create abstraction for access token related operations in authentication module #2435

Closed
7 tasks
jannyHou opened this issue Feb 20, 2019 · 5 comments
Closed
7 tasks

Comments

@jannyHou
Copy link
Contributor

jannyHou commented Feb 20, 2019

Description

Create abstraction for token service

  • Create abstraction for token service
// U stands for User model, in this story we can omit it and use `UserProfile` 
export interface Token Service<U extends Model> {}
  • Token service contains
    • generateAccessToken(user: U, options: Object): Promise<string>;
    • serializeAccessToken(response: Response): Promise<void>;
    • extractAccessToken(request: Request): Promise<string>;
    • verifyAccessToken(token: string): Promise<U>;
    • invalidateAccessToken?(token: string): Promise<boolean>;
  • Create unit tests Create Markdown file for acceptance tests (includes investigation of popular token-based authentication strategies and explain how this interface works with them).
@bajtos
Copy link
Member

bajtos commented Mar 5, 2019

// U stands for User model, in this story we can omit it and use `UserProfile` 
export interface Token Service<U extends Model> {}

I am concerned about requiring the U parameter to extend Model. What are the reasoning for requiring a Model class, instead of allowing app developers to use any interface?

Token service contains

  • generateAccessToken(user: U, options: Object): Promise<string>;
  • serializeAccessToken(response: Response): Promise<void>;
  • extractAccessToken(request: Request): Promise<string>;
  • verifyAccessToken(token: string): Promise<U>;
  • invalidateAccessToken?(token: string): Promise<boolean>;

I see two distinctive services in this list.

  1. A service to create/verify/invalidate access tokens. This service is independent on the transport mechanism used, e.g. a JWT token can be exchanged via REST, gRPC, GraphQL or even email!

  2. A service to serialize and deserialize the token depending on the transport used (serialize a token to a response, extract a token from a request).

Conceptually, the first service is operating at Data/Repository level, dealing with behavior related to data. The second service is operating at Transport/Controller level, dealing with transport-specific behavior.

I feel these two services are orthogonal and should be implemented independently, to make it easy to compose them in different ways. For example:

  • A single JWT service can be combined with different transport mechanisms - the token can be sent to the client via REST response body and read from Authorization header, but it can be also sent in Set-Cookie response header and read from request cookies.
  • A single transport mechanism (e.g. Bearer token) can be used with different access tokens - JWT, local AccessToken model, OAuth2 tokens, etc.

I am proposing to define two new abstractions:

  1. AccessTokenService

    • generateAccessToken(user: U, options: Object): Promise<string>;
    • verifyAccessToken(token: string): Promise<U>;
    • invalidateAccessToken?(token: string): Promise<boolean>;
  2. TokenTransportStrategy (feel free to come up with a better name)

    • serializeAccessToken(response: Response): Promise<void>;
    • extractAccessToken(request: Request): Promise<string>;

@bajtos
Copy link
Member

bajtos commented Mar 5, 2019

I am not sure how much sense it makes to implement TokenTransportService as a standalone entity, maybe it should be an implementation detail of each AuthenticationStrategy? Just an idea to consider.

@bajtos
Copy link
Member

bajtos commented Mar 5, 2019

serializeAccessToken(response: Response): Promise<void>;

If the serializer is writing the token to response body, then we need a way how to describe the response schema via OpenAPI. See also #2491 (comment)

@jannyHou
Copy link
Contributor Author

I am concerned about requiring the U parameter to extend Model. What are the reasoning for requiring a Model class, instead of allowing app developers to use any interface?

@bajtos This was aiming to auto generate the OpenAPI schema for the endpoint that returns a user profile. See code https://github.com/strongloop/loopback4-example-shopping/blob/master/src/controllers/user.controller.ts#L85, which hardcoded the schema in the shopping example as a workaround.
While now I realized it's not necessary. It's just for demo's purpose. In a valid use case, we can convert the user profile to a user model instance, or retrieve the user by userProfile's id.

I see two distinctive services in this list.
A service to create/verify/invalidate access tokens. This service is independent on the transport mechanism used, e.g. a JWT token can be exchanged via REST, gRPC, GraphQL or even email!
A service to serialize and deserialize the token depending on the transport used (serialize a token to a response, extract a token from a request).

Good catch! will split them into different services accordingly, like what we did for the login service, see PR #2576

@emonddr
Copy link
Contributor

emonddr commented Mar 28, 2019

implemented by PR #2576 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants