Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

Consider removing AllowAccessToAllScopes #499

Closed
leastprivilege opened this issue Nov 24, 2016 · 11 comments
Closed

Consider removing AllowAccessToAllScopes #499

leastprivilege opened this issue Nov 24, 2016 · 11 comments

Comments

@leastprivilege
Copy link
Member

leastprivilege commented Nov 24, 2016

No description provided.

@leastprivilege leastprivilege added this to the RC4 milestone Nov 24, 2016
@brockallen
Copy link
Member

done on config branch

@leastprivilege leastprivilege modified the milestones: RTM, RC4 Nov 30, 2016
brockallen added a commit that referenced this issue Nov 30, 2016
@brockallen
Copy link
Member

done

@brockallen brockallen modified the milestones: RC4-Update1, RTM Dec 2, 2016
@liamdawson
Copy link

Is there an alternative to be used?

@leastprivilege
Copy link
Member Author

Specify the scopes explicitly.

@ChristopherHaws
Copy link

ChristopherHaws commented Sep 1, 2017

I'm a bit confused by this. In the talk you gave at NDC (https://youtu.be/l7MgGY3lnts?t=30m59s) you mentioned that scope is no longer required and that this is why the scope field on RequestClientCredentialsAsync is not a required field (hence the null default value which is still the case). This is also mentioned in your blog (https://leastprivilege.com/2016/09/14/new-in-identityserver4-default-scopes/).

Is there a particular reason that this feature was removed? Perhaps a security risk of some sort? If this is the case, should the RequestClientCredentialsAsync method be changed to not allow a default null value?

Thanks!
Chris

@brockallen
Copy link
Member

scope is not required as a parameter, but you still need to indicate which scopes the client is allowed access to via the AllowedScopes.

@ChristopherHaws
Copy link

@brockallen After looking through the code trying to figure out why I've been getting an invalid_scope error, I found that the reason was because I was using AllowedGrantTypes = GrantTypes.ClientCredentials and I had AllowOfflineAccess = true.

I then looked up the RFC and found this section which states that a refresh token should not be included:
https://tools.ietf.org/html/rfc6749#section-4.4.3

Just wanted to post this here for anyone else running into this issue. Perhaps in the future additional error messages could be included in the server logs to indicate that the client is configured in an invalid state.

Thanks!

@brockallen
Copy link
Member

What was the error in the IdSvr logs for this misconfigured scenario?

@ChristopherHaws
Copy link

ChristopherHaws commented Sep 5, 2017

@brockallen After having read parts of the RFC, I guess the error isn't that bad, although it would be nice if the error happened earlier (at the time the client is configured) rather than during the actual client request... I just wasn't understand the error until I did some digging into the RFC to understand what some of these terms meant. I think that the part that was tripping me up was that I needed to understand that AllowOfflineAccess means allow the client to request a refresh token, which in turn doesn't make sense for the client credentials grant type. I am still fairly new to doing auth beyond using simple middleware for google/twitter/etc, so there are a lot of new terms that I am still trying to digest. :)

The error from the log:

IdentityServer4.Validation.TokenRequestValidator:Error: test-client cannot request a refresh token in client credentials flow
IdentityServer4.Validation.TokenRequestValidator:Error: {
  "ClientId": "test-client",
  "ClientName": "Test Client",
  "GrantType": "client_credentials",
  "Scopes": "test-scope",
  "Raw": {
    "grant_type": "client_credentials"
  }
}

@brockallen
Copy link
Member

IdentityServer4.Validation.TokenRequestValidator:Error: test-client cannot request a refresh token in client credentials flow

Yea, I am not sure how much more clear that can be. IdentityServer is not meant to absolve one from understanding the protocols :)

@lock
Copy link

lock bot commented Jan 14, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants