Support overriding of the default access/refresh token lifespans

ngrigoriev commented 5 years ago

Is your feature request related to a problem? Please describe.

We want different applications to have different lifespans of the access tokens. This is particularly important in the multi-tenant environments where the customer may have requirements different from our defaults.

Describe the solution you'd like

I think it would be great to have it done this way:

consent provider could have an optional parameter in the accept consent request allowing to override the default access/refresh token lifespans. This gives the maximum flexibility.
these override values can be (optional) configuration parameters of the clients, e.g. stored in the database along with other client parameters
if nothing is provided, then the global default value is used
(optional) as a safeguard, it would be great to have min/max allowed values for access/refresh token lifespans. If not provided, they could be initialized to the same values as default lifespan. This way a reasonable range could be enforced.

Describe alternatives you've considered

Multiple Hydra deployments - but this creates significant complexity and does not scale beyond 2-3 different tenants.

Additional context

N/A

aeneasr commented 5 years ago

While I wouldn't rule this out in general - the problem with setting this on a per-client basis is that some clients will use oauth2 access tokens as long living tokens or as a substitute for API keys - which is not a suitable use case for these types of tokens.

Why is the duration of the access token relevant if it can be refreshed any time?

ngrigoriev commented 5 years ago

I'd say it for us it is mostly about the customer requirements. We may think that 6 hours is a reasonable default but the customer may insist on 2 or 3 hours. This also gets a bit more sensetive when JWT token is used - since those cannot be truly revoked until they expire (from Oathkeeper's "jwt" authorizer standpoint, for example). So, I know that we may need to play the lifespan to meet some requirements.

On another side, we want to avoid having to refresh the tokens too often because we have some latency-sensetive apps and we hope to avoid the "cold start" latency during the anticipated user session period. But even this period may be different for different apps.

Bottom line: it is not about going to something low or something above a day - it is about having some freedom between, say, 2 hours and 8 hours for different apps.

The same applies to the refresh token.

And the reason why, I think, it may be cool to have that configurable at consent/login provider level is more or less the same: we have a cloud-based SaaS with multiple tenants. We do go through security review process with different customers and they, while understanding how OAuth2 works, may set the requirement for different access token lifespan for their users. Which means I may have one OAuth2 client that requires slightly different token lifespans for the users from different tenants.

aeneasr commented 5 years ago

Ok so what do you think about simply limiting access token lifetime to e.g. 7 days (which is way above what's recommended anyways) and then allowing for a per-client config (or alternatively allowing the consent app to define lifespan)?

ngrigoriev commented 5 years ago

I think this would do - basically, setting the max to the value determined by the environment and then being able to "lower" it via client config and consent app (nice to have). And I think it would be the ultimate flexibility if the same logic applies to both access and refresh tokens (independently, of course).

glerchundi commented 5 years ago

Ok so what do you think about simply limiting access token lifetime to e.g. 7 days (which is way above what's recommended anyways) and then allowing for a per-client config (or alternatively allowing the consent app to define lifespan)?

We didn't decided yet but some members of my team put on the table the possibility of changing lifetimes on a per client basis due to the different requirements each client have. I.e Web and iOS/Android apps.

Just putting my 2cents in.

aeneasr commented 5 years ago

Thank you :) As I've said before - if you have a refresh token the lifetime of an access token is negligible (in most cases) and best to keep reasonably short! In the scenario of web/native apps this definitely applies and you will have more complexity if you have different lifetimes as you will have when just following the refresh token flow.

timkelley-ingage commented 4 years ago

@aeneasr : I noticed that is issue is suggesting exactly what I had discussed around adding custom token expiration times (access_token_expiration, refresh_token_expiration) to the client data.

Here is the excerpt from pertaining to this from the separate issue:

This is not the case for ALL scenarios.. We have scenarios where we want to utilize the same Identity Provider (token minting) for different aspects of our business that require stricter security controls. With the ability to utilize the meta data we could generate tokens with different expiry times based on risk considerations of the data that they can retrieve as well as their "footprint" to our business. It doesn't make sense to stand up many Identity Providers with slightly different configurations as it just grows our security vectors without adding much benefit when it can be achieved in a single solution.

This piece is the only part of this discussion I don't necessarily agree with:

I think this would do - basically, setting the max to the value determined by the environment and then being able to "lower" it via client config and consent app (nice to have) I am not sure why the max would be determined by the environment. Why not just have a default for each expiration that is set by the environment, then override with values for the client?

aeneasr commented 4 years ago

I've tagged & labeled this issue so we can keep better track of it. I'm still not 100% convinced that this is neccesary / a good idea but it has been requested quite a lot. Maybe it would be easier to understand if you could share why exactly you need higher or lower times and what concrete outcome you expect of it?

ngrigoriev commented 4 years ago

Here is one use case that I can think of - real one for us. We have different API clients including some apps that are used by the call center agents. Their work day is usually around 8 hours or so. They start in the morning and they actively interact with the app (web app specifically) during the entire day. It is critical that the apps they use are working without any interruptions for the day.

Yes, I agree that the refresh, even in case of the simplest implicit grant, should be silent and non-intrusive. But in my case I am relying on the external IdP so when the refresh happens that IdP may break the "silent" refresh process and force the interactive logic - the agent will not be able to handle the call if suddenly he/she needs to put the password, probably 2FA...

Another case for us - API is one of our products. We have important enterprise customers who may have similar requirements. If we say that the token lifespan is 1 hour and the customer insists that they want to have it 4 hours - we do not necessarily want to argue too much. Yes, if they say 40 hours for the access tokens, we would really need to deal with it. But while within reasonable timeframe, we want to rather satisfy the customer's requerements but only in the scope of this customer, not having to change these lifespan values for everyone else, including ourselves.

aeneasr commented 4 years ago

Yes, I agree that the refresh, even in case of the simplest implicit grant, should be silent and non-intrusive. But in my case I am relying on the external IdP so when the refresh happens that IdP may break the "silent" refresh process and force the interactive logic - the agent will not be able to handle the call if suddenly he/she needs to put the password, probably 2FA...

As long as the session is not destroyed at ORY Hydra, the refresh will pass silently and is probably one of the few ways you could force a logout/ejection without having to wait 8 hours (user is banned or blocked or whatever).

Another case for us - API is one of our products. We have important enterprise customers who may have similar requirements. If we say that the token lifespan is 1 hour and the customer insists that they want to have it 4 hours - we do not necessarily want to argue too much. Yes, if they say 40 hours for the access tokens, we would really need to deal with it. But while within reasonable timeframe, we want to rather satisfy the customer's requerements but only in the scope of this customer, not having to change these lifespan values for everyone else, including ourselves.

Put the blame upstream and force your customer to use established security best practices ;) I'm happy to play the scapegoat.

No but all joking aside, I understand that this is a business requirement you're facing - but that doesn't justify a technical requirement upstream. I've explained why introducing this feature is problematic ("Oh I'll just set this expiry time to 30 days because I'm too lazy to think about refreshing right now") and ("Hey cool I can set this to whatever time I want per client so I'll just use this OAuth2 thing as an API Key and also other misconceptions"). And while I do understand that there's not much difference between 1 and 4 hours, you have to see it from the other side - the one that provides a generic solution for tens of thousands of deployments and that needs to make sure that no core security mechanism is misused/abused out of comfort.

chaopeng commented 4 years ago

We are also facing similar issue for refresh token:

Issue 1: Our identity broker should give refresh token lifespan no longer than upstream idps.

issue 2: not expandable refresh token is a requirement of the RFC. We are OAuth2 provider for some sensitive dataset, we have policy rules the maximum access time limitation. if refresh token is expandable, that breaks the policy.

Issue 3: When user choose remember information release preference (consent) for a duration, this means that the user only wants to give access for a limited time duration. The refresh token should has an expiry not longer than that. This is requirement maybe from GDPR. If service can still access out of consented time it may break gdpr lawful and fair processing.

aeneasr commented 4 years ago

We are OAuth2 provider for some sensitive dataset, we have policy rules the maximum access time limitation. if refresh token is expandable, that breaks the policy.

What do you mean with expandable?

Issue 1: Our identity broker should give refresh token lifespan no longer than upstream idps.

Refresh Tokens are not proof of authentication. They should not be considered when running Identity Brokers. The only thing that matters is the ID Token.

Issue 3: When user choose remember information release preference (consent) for a duration, this means that the user only wants to give access for a limited time duration. The refresh token should has an expiry not longer than that. This is requirement maybe from GDPR. If service can still access out of consented time it may break gdpr lawful and fair processing.

No, that is an incorrect understanding of what OAuth2 is and how it works. If the user does not want to grant a refresh token, he/she will deselect the "offline_access" scope. This has nothing to do with GDPR.

chaopeng commented 4 years ago

What do you mean with expandable?

I may read the code wrong. I seems like every time I refresh the refresh token, the lifespan got expanded. https://github.com/ory/fosite/blob/3ece795f3080db5de3529cea9bfa670e70704686/handler/oauth2/flow_refresh.go#L111

Refresh Tokens are not proof of authentication. They should not be considered when running Identity Brokers. The only thing that matters is the ID Token.

This make sense to me. I will rethink about this refresh token.

No, that is an incorrect understanding of what OAuth2 is and how it works. If the user does not want to grant a refresh token, he/she will deselect the "offline_access" scope. This has nothing to do with GDPR.

Let me double check if refresh token still require in our system. But refresh token lifespan longer than consent duration still odd to me.

Do you mean "offline_access" means allow access forever unless I revoke?

My understanding is: if user checked offline_access and remember for 10days. That means user allow application to grant refresh token and this refresh token can be only used for maximum 10 days.

aeneasr commented 4 years ago

My understanding is: if user checked offline_access and remember for 10days. That means user allow application to grant refresh token and this refresh token can be only used for maximum 10 days.

You are confusing authentication (login) with authorization (consent). Neither access nor refresh tokens have anything to do with "session".

morphar commented 4 years ago

I just started to implement Hydra + a custom IdP to be our single point of auth and ran into this exact issue.

We have an API that is accessed by: 1) SPA: Limiting the refresh_token to e.g. 4-6 hours seems to make sense 2) External services, that can be trusted with a refresh_token that never expires 3) External services, that should not be trusted with a refresh_token that never expires

In case 1, getting 4-6 hours more, on each access token refresh, until having to do a full login, makes sense and seems to adhere to the newest OAuth 2.0 for Browser-Based Apps draft.

MUST either set a maximum lifetime on refresh tokens OR expire if the refresh token has not been used within some amount of time.

It also provides a way of "trusting" the user, while being active.

The same can almost be done with silent refresh, remember cookies and no refresh token, but the cookie TTL is not being "reset", so an active user will need to re-log in at some point.

Either setting the TTL in the consent app or as a client setting would work. It think that setting it as a client setting would make most sense. This would make the initial creation of the client, the point where you choose how much this client can be trusted to keep secrets, while letting the consent app decide whether the login should be remembered or not.

Sorry for the rant, just wanted to add my experience and reasons for why different TTLs for at least refresh tokens, makes sense ;)

Thanks for a really nice piece of infrastructure that is otherwise even harder to get right. I appreciate your conservatism towards changes - it make sense, when dealing with security.

aeneasr commented 4 years ago

Thanks for writing this down @morphar :)

As always, it is important to keep this in mind:

An Access Token is a type of credential that can be used to execute a certain action. It is not correlated to a "session". Always think of Access Tokens in the terms of GitHub and CI (e.g. Travis or CircleCI). If you grant the CI access to your GitHub Repositories (using OAuth2), then the CI service has a GitHub access and refresh tokens. The CI will have those tokens for as long as they store them and they will be valid for as long as you don't revoke access for the CI in your GitHub profile. These tokens do not expire when you log out of GitHub. The CI will not log you out if you log out of GitHub. And GitHub will not log you out when you log out of the CI. The only thing this token is doing is telling GitHub: "Hey there so this user gave me this token and I would like to access repo ory/hydra. Please give me all the recent Commits". For GitHub, it does not matter that you're authenticated or not. There is valid proof of authorization not authentication.
An ID Token is proof of (recent - depending on implementation) authentication. It contains personal information (e.g. email or username). Let's go back to CI and GitHub - I know that GitHub doesn't issue ID Tokens at the moment but for the sake of simplicity let's say they do. So you perform the GitHub OAuth2 dance as above, but this time the CI also wants an ID Token back. It's the same flow but the tokens have different meaning. The ID Token, when it comes back to the CI, is used by the CI to create a session for the user agent (browser) that just completed the flow. That means that the CI has some type of possibility to bind a session to the user agent, most likely using a cookie. This cookie is the "authentication" credential for the CI. The CI has full control over the cookie. If you log out, the cookie is removed, hence authentication is revoked. If you reload the CI in the browser, you're not logged in any more. But the CI still has access to the repository using the Access Token.

Let's come to what you wrote:

SPA: Limiting the refresh_token to e.g. 4-6 hours seems to make sense

As I hopefully made clear, the Refresh Token is not a session replacement. I'm not sure what the reasoning is behind a short lived refresh token, but this seems to conflate session with authorization?

In the realm of OpenID Connect you can actually use something like "silent refresh" to synchronize the authentication session with upstream.

External services, that can be trusted with a refresh_token that never expires

This is a valid use case and implementable today. You can also limit the lifespan of refresh tokens so that applications that don't use the tokens have to re-authorize after a certain period of time (e.g. 30 days).

However, keep in mind that re-authorization is usually "silent". Notice how every time you hit that "Log in with GitHub" you never actually see GitHub, unless the connection is made for the first time? That's the same here!

So while it makes sense in certain cases to remove the tokens, the user won't actually notice anything (unless the user is logged out at GitHub in which case he/she would need to log in again).

External services, that should not be trusted with a refresh_token that never expires

This is a valid use case and implementable today. To get a refresh token, you need the offline or offline_access scope. If you (a) do not whitelist that scope for the OAuth2 Client or (b) the user does not consent to such scope, the application will not receive a refresh token.

Hope this helps.

morphar commented 4 years ago

Thank you so much for the detailed explanation @aeneasr :)

I might have understood things wrong (there's lots of conflicting opinions and information out there), but as far as I have understood it, using OAuth2 / OpenID for SPAs needs very strict flows, to ensure relatively high security.

What I'm doing right now (in my dev environment) is:

Do the initial OAuth2 dance, with a remember me cookie
Store access token - not refresh token
Do the "silent" re-authorization until the cookie expires
Re-direct to login, when cookie no longer exists

What I thought I could do:

Do the initial OAuth2 dance, without a remember me cookie
Store access token and refresh token
Do a silent access token refresh with refresh token regularly

What I would like to accomplish: Continuously allow access, while the user is active, without requiring login, unless x hours has passed with no activity or the browser was closed. This is mainly to ensure, that I can ensure, that a user's work is not lost, due to being redirected to login forms, etc. The current flow without refresh token, would accomplish this, if the remember cookie's expiry were updated on each "silent" re-auth.

That was why I wanted a short-lived refresh token, so that it could live in the browser, but without too much risk, if it were somehow obtained by a hacker. I would mitigate the above, by warning users of multiple active "sessions" or rather refresh tokens + checks on access from different IPs / geo locations within a short timeframe.

I'm not sure, that I'm thinking about this correctly or if the above is even doable in a way that makes sense with OAuth2 / OpenID?

fjvierap commented 3 years ago

@aeneasr We also require to do some extension to be able to configure different access token TTL per client. I understand the rationale behind keeping short TTLs and refresh it quite often instead of implementing this extension but unfortunately e cannot force all our client applications to do it this way :( . So, to archive this we will just configure a new access_token_ttl field in client metadata definition... Then adjust the code just adjust the expires_in field when this new field is configured in the client.

aeneasr commented 3 years ago

The clients you serve on your platform need to refresh tokens anyways - regardless of wether it's a long or short period. So why is the duration of a token relevant to them?

fjvierap commented 3 years ago

We have some cliente which requires longer TTLs than others because they need to limit the situation where async loading on a web page is running into 401s, esp. as the currently implemented fallbacks are not very graceful.

aeneasr commented 3 years ago

A longer TTL won't prevent that though, in my opinion. The client application should always anticipate that a 401 can happen and handle accordingly. Even if the TTL is 1 day, it can happen that the user hits it at exact that time. Or 1 hour, 1 minute, 1 week, 1 month.

ngrigoriev commented 3 years ago

There is one factor that is important here - the cost of user authentication when renewing the token (specifically not saying "refreshing"). If it was just a DB lookup, it would not be a problem. In my case, for example, it involves OIDC dialog with a customer's identity provider and this is where I have limited control over their preferences, which may be often expressed in form of their ID_TOKEN validity. Anyway, when the "cost" of user authentication is variable, the ideal frequency of the authorization token renewal may also become variable.

aeneasr commented 3 years ago

@ngrigoriev so you mean doing another oauth2 code grant?

ngrigoriev commented 3 years ago

@aeneasr Yes, but this is transparent for Hydra itself. My implementation of the login&consent provider, instead of validating the user credentials directly, initiates a OIDC flow towards the appropriate IdP (usually owned by the customer), OIDC magic happens, finally I get the ID_TOKEN from that IdP and accept Hydra's login request. I have some additional query parameters defined for the Hydra's authorization URL that control which "tenant" is involved and if I need to pass "prompt=none" for silent token acquisition. That's why my user authentication implementation is relatively expensive and involves 3rd party.

aeneasr commented 3 years ago

I see - and you don't have a refresh token to prolong the access tokens when they expire and need to reinit the code grant again?

ngrigoriev commented 3 years ago

@aeneasr Well, that's where the variations may start. The web apps that are SPA and cannot store the refresh token securely cannot use this grant to extend the access token. It could be done, in theory, but I would have to severely refresh the lifespan of the refresh token to an hour or a few hours in this case. Because I still want to make sure that I fail to issue the new access token for the user who has, for example, changed his/her password in the customer's IdP. As you can see, the problem is not that it cannot be arranged at all with the measures available. It cannot be arranged in the same way for different scenarios, different clients and tenants, this is why I was interested in this kind of feature.

Consider even simpler scenario. I would like to have different token lifespans for the API users going through the auth code (+PKCE) grant (interactive real users) and the services (client credentials grant). Yes, for the service2service client it is easy to get a new token, one POST to /token and you are done. But I do not need this client to renew their tokens every 30 minutes, while I may have to do it for the "interactive" ones. Simply because if I make it 2 hours for service-to-service clients, I have the tokens refreshed 4x less frequently. And for large number of clients it does make a difference.

aeneasr commented 3 years ago

I see your point. How could we prevent abuse though? My fear is that developers will try to work around refreshing completely and just set the TTL to infinity for the clients they are "too lazy" to fix. How can we enable your use case while prevent malpractice?

ngrigoriev commented 3 years ago

I believe that the healthy way would be to have the default token lifespan and the maximum lifespan. Both would be set by the administrator of the authorization service according to the organization security requirements (e.g. infinity will not be an option for the developers). So, for the individual application, the freedom would be only between the default and maximum. Personally, for our organizations, I would imagine the default being 15 minutes and the maximum - somewhere between 2-3 and 10 hours. And I would certainly not allow just any application to use the value different from the default.

fjvierap commented 3 years ago

We have done a change in https://github.com/ory/fosite for do not use the TTL configured globally but the one configured in client metadata if defined...

Something like:

accessTokenTTL := c.AccessTokenLifespan
    clientAccessTokenTTL := requester.GetClient().GetAccessTokenTTL()
    if clientAccessTokenTTL != 0 {
        accessTokenTTL = time.Duration(clientAccessTokenTTL) * time.Minute
    }

    responder.SetExpiresIn(getExpiresIn(requester, fosite.AccessToken, accessTokenTTL, time.Now().UTC()))

We did it for access token and refresh toke.. Please let me know if you consider that an valid option and I can create MR for it...

aeneasr commented 3 years ago

While I think that this is a good approach, I fear that this is not the most optimal solution - let me explain. OAuth2 clients are usually created by a third party / user / developer. They create the client, configure it, and so on. This goes even so far that we have OpenID Connect Dynamic Client Registration (https://openid.net/specs/openid-connect-registration-1_0.html) which is also supported by Ory Hydra. The idea here is that anyone is able to create such a client.

By exposing the mechanism of having the TTL configurable when creating such a client, a third party can work around security mechanisms employed by the Authorization Server by setting the TTL to something that's unreasonably long.

I think that setting the TTL to something non-default should probably be an administrative / configurable task. So something we do in the Ory Hydra config. There we could add a list of clients that have non-default TTLs for access tokens.

What do you think?

glerchundi commented 3 years ago

I’m in with this proposal of having a list of overriden TTLs per clientID. It could help us solving our SPA issues like the one it was mentioned in this thread.

Thanks!

On Fri, 5 Mar 2021 at 18:19, hackerman notifications@github.com wrote:

While I think that this is a good approach, I fear that this is not the most optimal solution - let me explain. OAuth2 clients are usually created by a third party / user / developer. They create the client, configure it, and so on. This goes even so far that we have OpenID Connect Dynamic Client Registration ( https://openid.net/specs/openid-connect-registration-1_0.html) which is also supported by Ory Hydra. The idea here is that anyone is able to create such a client.

By exposing the mechanism of having the TTL configurable when creating such a client, a third party can work around security mechanisms employed by the Authorization Server by setting the TTL to something that's unreasonably long.

I think that setting the TTL to something non-default should probably be an administrative / configurable task. So something we do in the Ory Hydra config. There we could add a list of clients that have non-default TTLs for access tokens.

What do you think?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ory/hydra/issues/1529#issuecomment-791561010, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARA7FRO6GE7YS4OTF3IFBDTCEHAJANCNFSM4IMB3F6Q .

aeneasr commented 3 years ago

I think that's probably the most reasonable path forward! If anyone wants to contribute this and needs pointers please let me know!

dadrus commented 3 years ago

@aeneasr: I somehow struggle to understand the proposal. Is the approach as described in https://github.com/ory/hydra/issues/1529#issuecomment-791242364 the desired way to go? So at the end, the administrator of hydra can override the global ttl defaults by client specific values along with setting the client_id, client_secret and so on properties. This shall only be possible for statically configured clients (via the admin api). Tokens issued to clients registered via dynamic client registration would always have the globally defined ttls. At the end, this is all about authorization of the corresponding request (does it come from the client itself or the admin) if the same client object can be created both ways. If my understanding is correct, we'll contribute :)

dadrus commented 3 years ago

I think I got your point @aeneasr. Your approach would be to put this list into the hydra.yaml. Correct? IMHO this will make the implementation more complex and it most probably will break abstractions. I don't know how the typical hydra deployment looks like in the wild, I however see it as follows: Since hydra is a security component, not everybody should have access to the admin end points. This should be only those people, who know, what they do and the effects, so usually a personnel responsible for the operation of hydra. To give the devs freedom to still be able to register clients (I don't think this happens without any communication), there are other APIs, which are available to devs for these purposes and which make just a subset of hydra API available (including what can be put into the payload). One such example is hydra-maester. If we talk about 3rd-parties, these shall not be able to create their clients as they want. The organization, operating hydra, will usually have some personal data of its customers and there is for sure a need for a contract between this organization and a third party, which data it is allowed to access (so which scopes the 3rd party client is allowed to request). This should be nothing a 3rd party can decide on its own. So the creation of clients will be a task executed the people operating hydra. If we talk about end customers, which use the services of an organization operating hydra, to which the organization way offer services, like kind of a dev portal to enable development of own clients for whatever purposes (e.g. like google offers), then there will be no direct access to hydras admin APIs. Instead there will be something, which the end customer will use, which will communicated with hydras admin APIs. And this something will offer only those options, which are acceptable from the organization point of view (usually also defined/influenced by security personnel of that organization).

aeneasr commented 3 years ago

The current Client endpoint follows OIDC DYnamic Client Registration which is, by design, a publicly available endpoint. Putting in any security controls there would be security madness!

The easiest way is probably still to do this during the consent step - even if I think that people will abuse that and hurt their security practices but in the end I kind of understand why some want this (even if I disagree in most use cases that it is necessary).

The idea with the config file is to have a mapping like

ttls:
  some-client:
    access_token: 2h
  another-client
    access_token: 10m

which means that we need the client IDs during config definition which theoretically limits sec ops to define this vs people making the REST request to consent endpoint. Then again, the consent endpoint allows you to specify the user that's signed in so if you don't trist those people in your org who can you trust 😅

dadrus commented 3 years ago

The current Client endpoint follows OIDC Dynamic Client Registration which is, by design, a publicly available endpoint. Putting in any security controls there would be security madness!

Fully agree with you! I'm however talking about something different - about different representations of the client object, depending on the used API. This way one would have more config options available and manageable over the admin API, but the dynamic client registration API would support only a subset of it (defined by the corresponding OIDC/OAuth2 specs), e.g. the client would not be able to set token ttls, as the corresponding end point would just ignore it, or response with an error. This way there are no limitations for sec dev ops and also no security issues at all. The approach with the config file mapping is in my opinion just a workaround to avoid introduction of such different representations (aka DTOs).

fjvierap commented 3 years ago

@aeneasr

The current Client endpoint follows OIDC DYnamic Client Registration

But it is still and administrative endpoint, correct?

admin.POST(ClientsHandlerPath, h.Create)

So it is only exposed publicly if the admin api is also exposed. Or did I misunderstand something?

aeneasr commented 3 years ago

Yeah but the idea is, if you want to expose it publicly, to more or less do so 1:1!

I think it probably just makes most sense from a developer perspective to have this be part of the consent API?

saich commented 3 years ago

My 2 cents:

Doing it in consent flow will imply it won't work with client_credentials flow in current system
For my POV, it makes sense for this to be part of client object in database (/clients endpoint). This could be optional field & null value in database would imply the application would use the global value for them, or can be specifically overridden per client as desired.

ngrigoriev commented 3 years ago

Personally, I do not like the idea of "override" config file. Simply because these values are client-specific and all client data sits in the DB and is accesiible/modifyable via APIs.

As for the consent provider vs client credentials grant (where consent provider is not called) - I do believe that some kind of consent-like callback (not user-facing) would be cool to have for that kind of flow. It would allow, for example, to inject additional metadata for the access token. And customize its lifespan too :)

aeneasr commented 3 years ago

As for the consent provider vs client credentials grant (where consent provider is not called) - I do believe that some kind of consent-like callback (not user-facing) would be cool to have for that kind of flow. It would allow, for example, to inject additional metadata for the access token. And customize its lifespan too :)

I understand the idea behind it! I think it is a tad too difficult (for the developer) to implement and also brittle as the flow is usually something being called in fast succession.

I think something like /config/clients/<id> would be preferable - it's a distinct API endpoint not exposed as OIDC Dynamic Registration and it would support the PUSH (rather PULL) model where the dev's backend updates the clients. This could allow specifying token TTLs for the different flows and also add client credential claims to the client.

dadrus commented 3 years ago

If I take a look at the current documentation (discovery endpoint): https://www.ory.sh/hydra/docs/next/reference/api/#openid-connect-discovery, I see the following:

{
  ...
  "registration_endpoint": "https://playground.ory.sh/ory-hydra/admin/client",
  ...
}

IMHO, this is a bug. The registration end point shall not be and administrative endpoint, there is a need for a public end point for the purpose of dynamic client registration. Otherwise one is forced to expose admin end points to the entire world, or at least the one, which can be used to register clients (if the routing infrastructure in front of hydra supports it). This is to invasive. So FMPOV, the proper solution would be to introduce public endpoints as described in OIDC Dynamic Client Registration specification. If we do so, we have a clear separation of concerns - specific end point(s) for public clients and specific end points for administrative purposes. And then, it is all about the proper representation of the client data for the one (public) and the other (admin) end points and what is possible via them.

This would be IMHO a clear approach with a clear implementation without any workarounds, which would require overwriting configuration or alike.

aeneasr commented 3 years ago

Thank you @dadrus - this appears however to be something not relevant to the original issue which is about access and refresh token epxiry. Regarding your question, please head over to: https://www.ory.sh/hydra/docs/production#exposing-administrative-and-public-api-endpoints

dadrus commented 3 years ago

Thank you for the link. I was somehow sure, there are two separate endpoints - one for dynamic client registration and the other for client administrative purposes. Hence my argumentation. Since this perception was wrong, I now understand your argumentation. Nevertheless, I personally see the original issue pretty much related to the currently ongoing discussion, even the original request was to move the corresponding logic to the consent provider. As long as there is just one endpoint, there is indeed actually only the one way to go which you've described above.

To avoid having further discussion in this ticket about a public endpoint for dynamic client registration and an administrative one for the actual administrative purposes, I'll open a new ticket.

github-actions[bot] commented 2 years ago

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers a year. That does not imply that the issue has no merit! If you feel strongly about this issue

open a PR referencing and resolving the issue;
leave a comment on it and discuss ideas how you could contribute towards resolving it;
leave a comment and describe in detail why this issue is critical for your use case;
open a new issue with updated details and a plan on resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneous you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

aeneasr commented 2 years ago

We're tackling this, see: https://github.com/ory/hydra/issues/3157

Closing this issue thus

ory / hydra

Support overriding of the default access/refresh token lifespans #1529