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

Best practice? Optional (single) endpoint authentication #4842

Closed
dougal83 opened this issue Mar 10, 2020 · 11 comments
Closed

Best practice? Optional (single) endpoint authentication #4842

dougal83 opened this issue Mar 10, 2020 · 11 comments
Assignees

Comments

@dougal83
Copy link
Contributor

Simply put, how is it best to create an endpoint that allows both anonymous and authenticated access. Should this pattern be avoided? Is there a better way?

Use case: the same interface to create a comment anonymously or with username appended.

Example (NOT FOR PRODUCTION):

  @authenticate('jwt-optional')
  @get('/whoami')
  @inject(SecurityBindings.USER) currentUser: UserProfile,
  whoAmI(): string {
    if (!currentUser.id) {
      return 'You are anonymous';
    }
    return this.userProfile[securityId];
  }

Optional JWT Strategy:

export class JWTOptionalAuthenticationStrategy
  implements AuthenticationStrategy {
  name = 'jwt-optional';

  constructor(
    @inject(TokenServiceBindings.TOKEN_SERVICE)
    public tokenService: TokenService,
  ) {}

  async authenticate(request: Request): Promise<UserProfile | undefined> {
    const token: string | undefined = this.extractCredentials(request);
    const userProfile: UserProfile = token
      ? await this.tokenService.verifyToken(token)
      : {[securityId]: ''};
    return userProfile;
  }

  extractCredentials(request: Request): string | undefined {
    if (!request.headers.authorization) return undefined;

    // for example : Bearer xxx.yyy.zzz
    const authHeaderValue = request.headers.authorization;

    if (!authHeaderValue.startsWith('Bearer')) {
      throw new HttpErrors.Unauthorized(
        `Authorization header is not of type 'Bearer'.`,
      );
    }

    //split the string into 2 parts : 'Bearer ' and the `xxx.yyy.zzz`
    const parts = authHeaderValue.split(' ');
    if (parts.length !== 2)
      throw new HttpErrors.Unauthorized(
        `Authorization header value has too many parts. It must follow the pattern: 'Bearer xx.yy.zz' where xx.yy.zz is a valid JWT token.`,
      );
    const token = parts[1];

    return token;
  }
}

tag: @emonddr

@emonddr
Copy link
Contributor

emonddr commented Mar 11, 2020

Umm. I would think an endpoint that requires authentication to protected resources shouldn't allow anyone on the internet to access that specific resource, so I am having trouble understanding the special scenario for such a need. Perhaps you can elaborate? thx.

@raymondfeng , do you have some thoughts on this? thx.

@dougal83
Copy link
Contributor Author

dougal83 commented Mar 11, 2020

Keyword being optional rather than required on the standard authentication via say JWT. A more sensible use-case would be using a separate token(say emailed to user for password reset) alongside the JWT bearer token. For example:

  @authenticate('jwt-optional')
  @patch('/users/{id}/password')
  async passwordUpdate(
    @requestBody({ // new password }}
    @inject(SecurityBindings.USER) currentUser: UserProfile,
    @param.path.string('id') id: string,
    @param.query.string('key') keyId?: string,
): User {
    if (!currentUser.id) {
      const keyFound = getKey(keyId);
      if (!keyFound) {
        // throw auth error
      }
    }

    const userId = currentUser.id ?? keyFound.id;
    // ... update user

    // ... return updated user (simplified, assuming password returned for example)
  }

Would this be recommended to completely encapsulate in an auth strategy? Or used with extreme caution.... Thanks for your advice.

@emonddr
Copy link
Contributor

emonddr commented Mar 13, 2020

@bajtos or @raymondfeng , what is your opinion on this scenario?

@dougal83
Copy link
Contributor Author

I'm thinking the following would be a move in a better direction... Would it be an idea to use SecurityBindings.SUBJECT to hold a one-time-key for a request? If anyone has an example to set that up I'd be a happy chap. :)

export class JWTOrKeyAuthenticationStrategy implements AuthenticationStrategy {
  name = 'jwt-key';

  constructor(
    @inject(TokenServiceBindings.TOKEN_SERVICE)
    public tokenService: TokenService,
    @inject(KeyServiceBindings.KEY_SERVICE)
    public keyService: AccountKeyService,
  ) {}

  async authenticate(request: Request): Promise<UserProfile | undefined> {
    const token: string | undefined = this.extractCredentials(request);
    const key: string | undefined = this.extractKey(request);

    if (!token && !key) {
      throw new HttpErrors.Unauthorized(
        'Authorization cannot find JWT or Key.',
      );
    }

    const accKey = key ? await this.keyService.getKey(key) : undefined;

    const userProfile: UserProfile = {
      [securityId]: '',
      ...(token && (await this.tokenService.verifyToken(token))),
      ...(accKey && {reqKey: accKey}),
    };

    return userProfile;
  }

  extractCredentials(request: Request): string | undefined {
    if (!request.headers.authorization) return undefined;

    const authHeaderValue = request.headers.authorization;

    if (!authHeaderValue.startsWith('Bearer')) return undefined;

    const parts = authHeaderValue.split(' ');
    if (parts.length !== 2) return undefined;
    const token = parts[1];

    return token;
  }

  extractKey(request: Request): string | undefined {
    if (!request.query['key']) return undefined;

    const key = request.query['key'];

    // account key is uuid with 36 chars
    if (key.length !== 36) {
      return undefined;
    }

    return key;
  }
}

@raymondfeng
Copy link
Contributor

@dougal83 Do you use authentication for personalization in this case?

@dougal83
Copy link
Contributor Author

dougal83 commented May 3, 2020

@dougal83 Do you use authentication for personalization in this case?

@raymondfeng Yes, that was the idea. Something non essential that could pull up customised data if available for the user.

@jannyHou
Copy link
Contributor

@dougal83 see issue #5310 and PR #5735, would that solve your problem here?

In your case, you can have 2 strategies separately JWTStrategy and KeyStrategy, and decorate your endpoint with

@authenticate(['jwt', 'key'])

@raymondfeng
Copy link
Contributor

I think #5735 will address the issue.

@dougal83
Copy link
Contributor Author

@jannyHou Yes, it looks like it will solve the issue supplying multiple strategies. Feel free to close this issue at your convenience.

@dhmlau dhmlau closed this as completed Sep 13, 2020
@dhmlau
Copy link
Member

dhmlau commented Sep 13, 2020

Closing per the above comment.

@nattyluke
Copy link

Its really much easier, you can give a 2nd argument like this to inject:
@inject(SecurityBindings.USER, {optional: true})
Works perfectly! Found there: #7079

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

6 participants