manfredsteyer / angular-oauth2-oidc

Support for OAuth 2 and OpenId Connect (OIDC) in Angular.
MIT License
1.88k stars 688 forks source link

Add Option sendIdToken similar to sendAccessToken #1059

Open yelhouti opened 3 years ago

yelhouti commented 3 years ago

Is your feature request related to a problem? Please describe. I need to re-implement the interceptor while changing only with accessToken with idToken.

Describe the solution you'd like Have two booleans, and send the appropriate token depending on the one that is true.

Describe alternatives you've considered Have an enum, NONE, ACCESS_TOKEN, ID_TOKEN (or undefined instead of none).

I can make a PR if needed

jeroenheijmans commented 3 years ago

Hmm, I'm not so sure about if this is a good idea to implement. This post from Auth0 eloquently describes my thoughts, summarized:

Could you elaborate why you would need this feature? Maybe there's a use case I'm missing?


EDITs per April 18th, adding some references after researching a bit more:

Microsoft Azure AD documentation:

ID Tokens should be used to validate that a user is who they claim to be and get additional useful information about them - it shouldn't be used for authorization in place of an access token.

Auth0 Tokens documentation:

ID Tokens should never be used to obtain direct access to APIs or to make authorization decisions.

IDS4 documentation on Identity Token and Access Token:

An identity token represents the outcome of an authentication process. It contains at a bare minimum an identifier for the user (called the sub aka subject claim) and information about how and when the user authenticated. It can contain additional identity data. ... An access token allows access to an API resource. Clients request access tokens and forward them to the API.

Okta's blog (their docs are in line but less precise about his subject):

Access tokens are used as bearer tokens. A bearer token means that the bearer can access authorized resources without further identification. ... Access tokens are used to gain access to resources by using them as bearer tokens

yelhouti commented 3 years ago

@jeroenheijmans thank you for taking the time.

The token I retrieve from my IdP is used to authenticate the user, and allow specific actions depending on who he is. This token is sent to the backend with each request. Using an access token in this case would be a very bad idea as you know.

The other alternative I have is using the authorization flow with my server and use cookies between my frontend and backend but I would prefer not to do that, (stateless ...).

jeroenheijmans commented 3 years ago

allow specific actions ..... Using an access token in this case would be a very bad idea

Hmmm, I think the reverse is true. Access tokens are precisely meant for authorization (so allowing access to resources). Your API should require access tokens for authorization, not identity tokens.

Note that access tokens can contain scopes, audiences, claims: those are not about "who a user is" (identity), but "what they are allowed to do" (access).

yelhouti commented 3 years ago

@jeroenheijmans Audience is a field specific to idToken. It is the main difference between OAuth and OIdC . You should never use an access token to authenticate a user. IE perform an access on you website thinking the user who provided the access token is the one using it. The meaning od Authorization in the spec if for data retrieval example authorization to view profil info picture email...

https://openid.net/specs/openid-connect-core-1_0.html#:~:text=The%20OpenID%20Connect%20Core%201.0,considerations%20for%20using%20OpenID%20Connect.

jeroenheijmans commented 3 years ago

I think we'll have to agree that we disagree 😅

yelhouti commented 3 years ago

@jeroenheijmans ok let's keep discussion here :p

yelhouti commented 3 years ago

Please give me a chance to prove what I say: The main difference between access and idToken is the required audience field can we agree on this ?

yelhouti commented 3 years ago

@jeroenheijmans if you are not willing to discuss this can you point me to the right person to talk to, this also seems like a security risk to me

jeroenheijmans commented 3 years ago

Sorry, I didn't mean to be rude, but I think we fundamentally understand the specs differently. Don't think more discussion would lead anywhere. - I realize I could also be wrong, if I would've been the one making the final call and/or building this, I'd certainly dive deeper into the specs. But I also have limited time for moderating open source, and since you can easily write an interceptor to add this functionality yourself, I'm not keen doing so at the moment. Hope you understand?


As for alternatives:

For this specific library, I am currently probably the one to talk to 😅 (in addition to its maintainer, but they don't do too much day-to-day community support), as I'm the one doing about 98% of all direct replies and moderation.

You could wait for a second opinion from the maintainer, but they reserve typically at most 1 or 2 times per year for updates, there's quite a backlog of open issues and PRs as you can see.

If you really want to stick to this library and get this fixed, I highly recommend forking and making your own changes.

There's also very good alternatives, e.g. Auth0's clients, or for Angular specifically there's damienbod/angular-auth-oidc-client. You could see if they do have native support for sending ID Tokens to the API?

As a final alternative, there's always an option to write your own HttpInterceptor, so you can easily do it in your app without changing the libraries.


Hope that helps you!

I'll leave this issue open for a little while more, see if the maintainer has time to jump in and change perspectives. Otherwise I might close it some time later. We'll see.

yelhouti commented 3 years ago

@jeroenheijmans thank you for taking the time I really appreciate it.

As you mentioned I added an interceptor, so we could end this here, by me thanking you for all the time you are spending on this open source project. But I am afraid that the advice given by you to other users of this library exposes them to a huge security risk. I really don't mean to be rude, but I am here referring to your comment in : https://github.com/manfredsteyer/angular-oauth2-oidc/issues/1060#issuecomment-820380481

The access token can be sent to reach secured APIs (which can then use that token to get Identity information about the user, if they really need to)

What I understand from your answer is that the server exposing the API, will use the token to figure out who is sending the request, and allow certain actions.

If this is not the case, please excuse me.

If on the other hand this is the case, I beg for some few minutes of your time, I have read extensively about OAuth2 and OIdC (specs included) and would like to give an easy example of why this should not be done.

First, the access_token by the default does not have an aud (audience) field, and even when it's there it is not checked by the resource server (by default).

Let's suppose we have one malicious resource server (some app that adds filters to you facebook photos) let's call it filterApp. The filterApp can access you photos because you provided here with an access_token from facebook. And let's suppose that I have an app like dropbox (let's call it myshare) that uses access access_tokens from facebook to retrieve users emails, create an account, and add/delete files...

The owner of the malicious app could copy the access_token that hes app received from one of his users, and use it to authenticate in myShare to view/modify/delete his files...

myShare has no way no way of knowing that the access_token is stolen because it doesn't have an aud field, if it has one, it would say filterApp and myShare would now not to accept it.

What I am explaining here, is the reason how and why OIdC was added on top of OAuth2. If you are interested I can link for IT Security papers explaining what I just said.

To be clear, Facebook Login is using OIdC (or a variant of it) and is not vulnerable to this attack, unless your server doesn't check the aud field in the token to match its clientId...

Thanks again for the work you are putting into this ! Cheers

jeroenheijmans commented 3 years ago

Thanks for your thoughtful reply.

We will remain in disagreement I think, as I'll stick with above-linked resources from my comments. Since you feel otherwise, I recommend trying one of the alternative approaches (interceptor, another library, forking).

Kind regards!

yelhouti commented 3 years ago

Thanks again for reading, and I understand that I didn't answer all the points in your previous comments (I didn't see the edit) so I will do it for future readers, and maybe it will be of help to you too.

1- Auth0 talks about direct access to an API, which is not what most devs do here, we provided access through an angular app, the user authenticates and get a token that proves his identity (roles...) forwards the token the server, that may use these info, to grand access depending on the roles or the user. They also state that if "So unless you are in control of both the client and the API, sending an id_token to an API will not work." which is the opposite of what most of us do, we build a client for an API that we control.

2- on the other hand, they all clearly state that who ever "Bearer" has an access token can access the resource, and they should not be used for authentication, the access_token contains all information about what I can and cannot do.

When I build an angular app, and I send a token to an API (specific to that app), if it uses the access_token to know who I am, it is not authorizing me, it is authenticating me, BIG difference and problem.

I understand now where the confusion comes from, and as you mentioned maybe we will remain in disagreement. But please do not tell users of the library who ask about id_token to use access_tokens instead, they must have read somewhere that they should use id_tokens and it's for a reason.

CedricHg commented 3 years ago

I have a possible case where sending the id token to the backend could maybe be convenient. The application connects to a .NET Framework SignalR service. Each time the access token is renewed, the connection to SignalR must be closed and reopened with this new token.

In the backend we get the userinfo via the relevant oidc endpoint as per usual. However a "bug" we notice here is that if the user has the application open in multiple tabs, all of the SignalR reconnection requests trigger simultaneously, and we spam the server with multiple userinfo calls - as the requests run simultaneously, for multiple of those nothing is in cache yet.

Given that the token is renewed every 3 minutes by default, and some users have the application open in more than a dozen tabs, we have users that spam 200 userinfo requests in 15 minutes.

Admittedly this is a flaw in the backend, fixing this would require some kind of multithreaded keyed lock system, but it seems to me sending both an access token for authorization and an id token for identification is a simpler fix that would solve this case, unless I have misunderstood something.

jeroenheijmans commented 3 years ago

@CedricHg Ooh, that sounds like a hard case indeed.

Some specific things that stood out to me in your post:

sending both an access token for authorization and an id token for identification is a simpler fix

All sources I could find recommend against it but if you are invested in this as a fix, I recommend using a custom interceptor.

that the token is renewed every 3 minutes by default

That sounds like a pretty harsh time span for expiration, I don't know your context or security requirements, but typically 60 minutes for an access token is what I see.

have the application open in more than a dozen tabs

There are many different ways that tabs in a browser can communicate with each other, either directly or possibly with OIDC's "Session Checks" via the IDS, that will probably solve your situation in a possibly more convenient way?


Either way, thanks for sharing your scenario, as it (plus any replies) might help others in a similar situation.

phthatcher-conocophillips commented 3 months ago

TLDR: ask was to be able to send id token instead of access token using a config option, this does that.

Short version, during init of your angular app override the oauthService.GetAccessToken function to return the id token instead or the access token, simple implementation, cant guarantee forward compatibility, Easier than implementing your own HttpInterceptor though. in your app.compotent.ts: constructor(private oauthService:OAuthService) { this.oauthService.getAccessToken = () => this.oauthService.getIdToken(); }

Job done. If you configure the lib to send Bearer token using NgModule imports: [ ... OAuthModule.forRoot({

  resourceServer: {
    sendAccessToken:true 
  }
} as OAuthModuleConfig)

],

the native lib function to get authorizationHeader calls getAccessToken() which has been overridden on your init, now every http request sends a id token instead of an access token

Story: I'll admit, this is a hack that worked for my scenario, not applicable to most, but I'll put it out here for those who may be facing similar issues. Again, not ideal, it isn't backwards compatible etc because it overrides a native lib function, but it is easy to implement, and honestly if you are doing front-end OIDC instead of back-end OIDC, its probably something you are looking for.

My scenario, I need username, first/last name, email address, active directory groups, these determined user roles in the app, and previously were obtained by an IIS webserver using windows authentication to get HttpContext. No longer available considering app moved to a new domain, different domain controllers, different security requirements, but still I needed it to be backwards compatible to existing app, which used username and/or email addr as primary key. Existing back-end required a modified [Authorize] component for role based auth and I didn't want to unwind all of that during this move.

Using angular front-end (SPA), c# web api (4.7, not 'core') back-end, app now has an MFA requirement based on OIDC, but previously already has role based authentication built into back-end that is a combination of active directory (IIS authentication required) and SQL (for extended role authentications without requiring AD)