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

Extend authentication decorator to apply security to operations #4990

Closed
dougal83 opened this issue Mar 29, 2020 · 8 comments
Closed

Extend authentication decorator to apply security to operations #4990

dougal83 opened this issue Mar 29, 2020 · 8 comments

Comments

@dougal83
Copy link
Contributor

Suggestion

Extend @authenticate decorator to apply security to operations. Initial iteration could start with http authentications schemes and possibly extended to rudimentary base apiKey scheme implementation. A caveat is that a securitySchemes object must be provided by the authentication strategy.

Further expansion later for other schemes in the auth specification docs.

Related:
#4416 feat: adds @tags convenience decorator
#4693 feat(Authentication) enable authentication strategies to contribute OASEnhancer

Acceptance criteria

TBD - will be filled by the team.

@Salketer
Copy link

Salketer commented Apr 3, 2020

I was looking for this. As you say #4693 allows the strategy to define the scheme to the specs. I think that every @authenticate('strategyName') should allow the target strategy to merge its security spec to the decorated path.

@jannyHou
Copy link
Contributor

jannyHou commented Apr 3, 2020

@dougal83 Thank you for opening this feature. Improving @authenticate() is one good approach.

Another approach to consider: introducing a new decorator @security() in @loopback/openapi-v3. This way the generated OpenAPI security spec is not coupled with the @loopback/authentication.

E.g.

// specify a name then the enhancer fills in the spec
@security('jwt')
// or provide the spec directly
@security(security_spec_object)
@authentication('jwt')
whoAmI(@inject(SECURITY_BINDINGS.USER)user: UserProfile) {
  console.log(this.user)
}

@jannyHou
Copy link
Contributor

jannyHou commented Apr 3, 2020

And for @Salketer 's suggestion 👍 :

I was looking for this. As you say #4693 allows the strategy to define the scheme to the specs. I think that every @authenticate('strategyName') should allow the target strategy to merge its security spec to the decorated path.

Yes in this case the enhancer could infer and fill in the spec by strategyName. But the controller spec generator still needs to process the auth metadata first: read the auth metadata and add strategyName to an extension field like x-authentication-name, or merge the spec directly into an operation's security field.

@dougal83
Copy link
Contributor Author

dougal83 commented Apr 5, 2020

Another approach to consider: introducing a new decorator @security() in @loopback/openapi-v3. This way the generated OpenAPI security spec is not coupled with the @loopback/authentication.

@jannyHou Personally I'd like to see both the @security() decorator and an extension of the @authenticate() functionality as described above. Negating the need to add unnecessary markup for a feature that naturally follows is a plus, especially when it is a key feature that will be used repetitively.

@deepakrkris
Copy link
Contributor

@jannyHou could you please fill in the acceptance criteria for this issue ?

@nflaig
Copy link
Member

nflaig commented Jan 24, 2021

@dougal83 @raymondfeng what do think about extending the AuthenticationStrategy interface with an optional method which allows strategies to provide the security spec and then a spec enhancer in the authentication module would update the openapi spec based on the authentication metadata, see gist for a draft how this could look like

@stale
Copy link

stale bot commented Aug 1, 2021

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

@stale stale bot added the stale label Aug 1, 2021
@stale
Copy link

stale bot commented Aug 31, 2021

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

@stale stale bot closed this as completed Aug 31, 2021
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

5 participants