Repository: sebadob/rauthy (language: rust)

Issue:
Unable to redeem refresh_token
I'm using oauth2-proxy together with rauthy to self-host, but I have issues redeeming refresh_tokens, as I get an error: {"error": "Unauthorized"}, with now apparent further explanation in the logs.

I use Rauthy ghcr.io/sebadob/rauthy:0.26.2-lite (I am unable to upgrade as of yet), and to try to make a more minimal reproduction, I was able to trigger the same behaviour easily using [oauth2c](https://github.com/cloudentity/oauth2c/releases/tag/v1.17.0).

Is there anything I may have missed with regards to refresh_tokens that may be the reason why I am not able to redeem them? Is there a rate-limit? or dont-use-before or maybe I'm doing something obviously wrong? I can make a more automated minimal reprodution if needs be, just wanted to get the question out before in case it is something obvious I have missed.

```
# scripts and commands needs oauth2c and jq installed
# run ghcr.io/sebadob/rauthy:0.26.2-lite locally
# set up client as screenshots below show
# update the client-secret in the scripts below
# then execute these two scripts in succession
./start_session.bash > tokens.json
./refresh_session.bash $(cat tokens.json | jq -r '.refresh_token')
```


#### start_session.bash
```bash
oauth2c \
    --client-id test \
    --client-secret '7mLGUT2bfyA0Gln0b22j9dEks60WBEXnjyalFVxQ4zXMTmkQNET7bTFNzwNdasDk' \
    --insecure \
    --pkce \
    --scopes "openid profile email offline_access" \
    --response-types code \
    --response-mode query \
    --grant-type authorization_code \
    --auth-method client_secret_post \
    https://localhost/auth/v1
```
#### refresh_session.bash
```bash
oauth2c \
    --client-id test \
    --client-secret '7mLGUT2bfyA0Gln0b22j9dEks60WBEXnjyalFVxQ4zXMTmkQNET7bTFNzwNdasDk' \
    --insecure \
    --pkce \
    --scopes "openid profile email offline_access" \
    --response-types code \
    --response-mode query \
    --grant-type refresh_token \
    --auth-method client_secret_post \
    --refresh-token $1 \
    https://localhost/auth/v1
```


This is my client setup

![image](https://github.com/user-attachments/assets/76ef8ee2-6588-43bd-9dab-8fedb0f50b0a)
![image](https://github.com/user-attachments/assets/706e6332-e9b8-4629-a3e0-f1eb071db9b2)



Discussion / hints:
How exactly are you trying to redeem it, with which kind of request?

Rauthy is super strict about refresh tokens. No matter what the outcome of the request is, if it turns out to be a successfuly request or a malformed request, you can only use any refresh token once, then it's gone. Even further, when you use a refresh token for an existing session either before its validity, after it has expired, or in combination with any malformed request, not only the token itself will be deleted, but also the whole session with all linked tokens will be invalidated as well, just in case a token leaked somehow on a client.
I redeem it like I described in the post, calling the scripts in with ~10 seconds in between (meaning the refresh token might only be 10 seconds old). Is there a minimum time I must wait for it to be redeemable perhaps?

Or do you mean how oauth2-proxy redeems it? I was hoping I have triggered the same sad-path using the setup above in the post as oauth2-proxy triggers, but there is of course a chance for that not to be the case.
>  Is there a minimum time I must wait for it to be redeemable perhaps?

Yes. The refresh token comes with an `nbf` claim and it will only become valid 60 seconds before the current access token expires. If you use it before it's valid, Rauthy considers this an invalid usage and will delete everything that is somehow linked to this token + all other sessions and all other refresh tokens.

The reason that there is an `nbf` is to prevent simple refresh token spam whenever a client needs to do something, which is in the end just a very bad and lazy implementation, because the access token will still be valid at that point.

Edit:

If you reduce the access token lifetime for your test to 60 seconds, it should work right away.
Damn, yeah. That is probably my issue. Is there a way to relax this behaviour somehow? I thought the refresh_token was opaque to the client, and unfortunately oauth2-proxy does not support inspecting the lifetime of access/id tokens.

Regardless, I am pretty sure this is my problem, thanks for the info!

EDIT: Yup, that was the problem, thanks!!
> Is there a way to relax this behaviour somehow?

Not yet, because is was never needed. It would be very simple to make this modifyable though, it's only a single line of code: https://github.com/sebadob/rauthy/blob/85ad0959380860e1d912b52808a6a2ad20df18f4/src/service/src/token_set.rs#L385

> I thought the refresh_token was opaque to the client

It can be considered as opaque, but when oauth2-proxy is trying to refresh when it still has a valid access token, its probably worth an issue there, because it should inspect and respect at least the `exp` of the access token and why should you refresh all the time and waste ressources and time when it's completely unnecessary.

Another solution then would be, if it refreshes all the time anyway, to keep the access token lifetime low enough that it does not produce issues, or increase the session lifetime on Rauthy and don't issue refresh tokens. Then it would redirect you each time but with a still valid session will get an new token set without user interaction.

Btw, you only get the very opaque unauthorized error to not leak any information to possible attackers.
> > Is there a way to relax this behaviour somehow?
> 
> Not yet, because is was never needed. It would be very simple to make this modifyable though, it's only a single line of code:
> 
> https://github.com/sebadob/rauthy/blob/85ad0959380860e1d912b52808a6a2ad20df18f4/src/service/src/token_set.rs#L385

Ok. Just knowing about the NBF helps immensely =)

> > I thought the refresh_token was opaque to the client
> 
> It can be considered as opaque, but when oauth2-proxy is trying to refresh when it still has a valid access token, its probably worth an issue there, because it should inspect and respect at least the `exp` of the access token and why should you refresh all the time and waste resources and time when it's completely unnecessary.

I agree, and this has and is being discussed over in that project (e.g https://github.com/oauth2-proxy/oauth2-proxy/issues/1836), but AFAIK there is not yet consensus on how to do this, and no one have yet provided an acceptable PR. Also there is other code-base-spanning work in progress in the project that makes it very inconvenient ATM (refactoring of configuration).
 
> Another solution then would be, if it refreshes all the time anyway, to keep the access token lifetime low enough that it does not produce issues, or increase the session lifetime on Rauthy and don't issue refresh tokens. Then it would redirect you each time but with a still valid session will get an new token set without user interaction.

Yeah, the first one sounds very palatable in my case, since the amount of users is expected to be very low.

> BTW, you only get the very opaque unauthorized error to not leak any information to possible attackers.

Yeah, that was what I thought.

In case you are interested in some ideas of how to make this easier to debug:
- Maybe the error could be more detailed for clients using both ID and secret (confidential clients)
- Maybe the debug log could show invalidation of sessions together with the reason when in debug or higher verbosity
- Maybe document the refresh token NBF in the rauthy docs and example configuration

Thanks for the assistance! I'll close this issue since I am indeed able to redeem the refresh token =D
Oh don't close yet, I was about to add an option to solve your issue. ^^
Will come anyway.
Ok! Feel free to close it when you feel like it. I unfortunately can't assist with testing the feature for a while though, since I am kind of stuck on the release before the bump of the OpenAPI spec version for a while ^_^"
There is already additional logging in such an error case, but it's in either `debug` or `trace` level, not sure about it. I can bump this up a little bit and at least lot some information without leaking secrets to help debugging.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6a3b713b9..b670c3362 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,35 @@
 # Changelog
 
+## UNRELEASED
+
+### Changes
+
+Even though not recommended at all, it is not possible to opt-out of the `refresh_token` nbf claim, and disable it.
+
+By default, A `refresh_token` will not valid before `access_token_lifetime - 60 seconds`, but some (bad) client
+implementations try to refresh `access_tokens` while they are still valid for a long time. To opt-out, you get a new
+config variable:
+
+```
+# By default, `refresh_token`s will have an `nbf` claim, making them valid
+# at `access_token_lifetime - 60 seconds`. Any usage before this time will
+# result in invalidation of not only the token itself, but also all other
+# linked sessions and tokens for this user to prevent damage in case a client
+# leaked the token by accident.
+# However, there are bad / lazy client implementations that do not respect
+# either `nbf` in the `refresh_token`, or the `exp` claim in `access_token`
+# and will refresh early while the current access_token is still valid.
+# This does not only waste resources and time, but also makes it possible
+# to have multiple valid `access_token`s at the same time for the same
+# session. You should only disable the `nbf` claim if you have a good
+# reasons to do so.
+# If disabled, the `nbf` claim will still exist, but always set to *now*.
+# default: false
+DISABLE_REFRESH_TOKEN_NBF=false
+```
+
+[]()
+
 ## v0.27.1
 
 ### Bugfix
diff --git a/Cargo.lock b/Cargo.lock
index 59feaa972..dcacdfd11 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2464,8 +2464,9 @@ dependencies = [
 
 [[package]]
 name = "hiqlite"
-version = "0.3.1"
-source = "git+https://github.com/sebadob/hiqlite.git#05a89f8d1f2f88a056d5b055f3471140cbfca260"
+version = "0.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4278d3e5e0093f7da889443336a82aef39d57d286ac6a0d6933ec45d1bd4dd3b"
 dependencies = [
  "argon2",
  "axum",
diff --git a/book/src/config/config.md b/book/src/config/config.md
index f086c72b8..e137740dc 100644
--- a/book/src/config/config.md
+++ b/book/src/config/config.md
@@ -62,6 +62,22 @@ deploying with Kubernetes, extract these values into Kubernetes Secrets.
 # default: false
 #DANGER_DISABLE_INTROSPECT_AUTH=false
 
+# By default, `refresh_token`s will have an `nbf` claim, making them valid
+# at `access_token_lifetime - 60 seconds`. Any usage before this time will
+# result in invalidation of not only the token itself, but also all other
+# linked sessions and tokens for this user to prevent damage in case a client
+# leaked the token by accident.
+# However, there are bad / lazy client implementations that do not respect
+# either `nbf` in the `refresh_token`, or the `exp` claim in `access_token`
+# and will refresh early while the current access_token is still valid.
+# This does not only waste resources and time, but also makes it possible
+# to have multiple valid `access_token`s at the same time for the same
+# session. You should only disable the `nbf` claim if you have a good
+# reasons to do so.
+# If disabled, the `nbf` claim will still exist, but always set to *now*.
+# default: false
+#DISABLE_REFRESH_TOKEN_NBF=false
+
 # Can be used when 'OPEN_USER_REG=true' to restrict the domains
 # for a registration. For instance, set it to
 # 'USER_REG_DOMAIN_RESTRICTION=gmail.com' to allow only
diff --git a/rauthy.cfg b/rauthy.cfg
index ea9427419..250b314c1 100644
--- a/rauthy.cfg
+++ b/rauthy.cfg
@@ -57,6 +57,22 @@ USERINFO_STRICT=true
 # default: false
 #DANGER_DISABLE_INTROSPECT_AUTH=false
 
+# By default, `refresh_token`s will have an `nbf` claim, making them valid
+# at `access_token_lifetime - 60 seconds`. Any usage before this time will
+# result in invalidation of not only the token itself, but also all other
+# linked sessions and tokens for this user to prevent damage in case a client
+# leaked the token by accident.
+# However, there are bad / lazy client implementations that do not respect
+# either `nbf` in the `refresh_token`, or the `exp` claim in `access_token`
+# and will refresh early while the current access_token is still valid.
+# This does not only waste resources and time, but also makes it possible
+# to have multiple valid `access_token`s at the same time for the same
+# session. You should only disable the `nbf` claim if you have a good
+# reasons to do so.
+# If disabled, the `nbf` claim will still exist, but always set to *now*.
+# default: false
+#DISABLE_REFRESH_TOKEN_NBF=false
+
 # Can be used when 'OPEN_USER_REG=true' to restrict the domains for a registration. For instance, set it to
 # 'USER_REG_DOMAIN_RESTRICTION=gmail.com' to allow only registrations with 'user@gmail.com'.
 # default: ''
diff --git a/src/api/src/oidc.rs b/src/api/src/oidc.rs
index 32b120b61..ae9e53a49 100644
--- a/src/api/src/oidc.rs
+++ b/src/api/src/oidc.rs
@@ -907,6 +907,7 @@ pub async fn post_token(
             Ok(resp)
         }
         Err(err) => {
+            error!("{}", err.message);
             if !has_password_been_hashed {
                 return Err(err);
             }
diff --git a/src/common/src/constants.rs b/src/common/src/constants.rs
index f5eb7ed08..56e07bbdd 100644
--- a/src/common/src/constants.rs
+++ b/src/common/src/constants.rs
@@ -170,6 +170,10 @@ lazy_static! {
         .unwrap_or_else(|_| String::from("false"))
         .parse::<bool>()
         .expect("DANGER_DISABLE_INTROSPECT_AUTH cannot be parsed to bool - bad format");
+    pub static ref DISABLE_REFRESH_TOKEN_NBF: bool = env::var("DISABLE_REFRESH_TOKEN_NBF")
+        .unwrap_or_else(|_| String::from("false"))
+        .parse::<bool>()
+        .expect("DISABLE_REFRESH_TOKEN_NBF cannot be parsed to bool - bad format");
 
     pub static ref SEC_HEADER_BLOCK: bool = env::var("SEC_HEADER_BLOCK")
         .unwrap_or_else(|_| String::from("true"))
diff --git a/src/common/src/lib.rs b/src/common/src/lib.rs
index ff4894efc..bfb3cd2c8 100644
--- a/src/common/src/lib.rs
+++ b/src/common/src/lib.rs
@@ -1,7 +1,7 @@
 // Copyright 2024 Sebastian Dobe <sebastiandobe@mailbox.org>
 #![forbid(unsafe_code)]
 // needed because the lazy_static! initialization of constants grew quite a bit
-#![recursion_limit = "256"]
+#![recursion_limit = "512"]
 
 use crate::constants::DB_TYPE;
 use std::env;
diff --git a/src/service/src/token_set.rs b/src/service/src/token_set.rs
index 96a720349..e204579c0 100644
--- a/src/service/src/token_set.rs
+++ b/src/service/src/token_set.rs
@@ -5,7 +5,8 @@ use jwt_simple::claims::Claims;
 use jwt_simple::prelude::{coarsetime, UnixTimeStamp};
 use rauthy_api_types::oidc::JktClaim;
 use rauthy_common::constants::{
-    DEVICE_GRANT_REFRESH_TOKEN_LIFETIME, ENABLE_SOLID_AUD, ENABLE_WEB_ID, REFRESH_TOKEN_LIFETIME,
+    DEVICE_GRANT_REFRESH_TOKEN_LIFETIME, DISABLE_REFRESH_TOKEN_NBF, ENABLE_SOLID_AUD,
+    ENABLE_WEB_ID, REFRESH_TOKEN_LIFETIME,
 };
 use rauthy_common::utils::base64_url_no_pad_encode;
 use rauthy_error::{ErrorResponse, ErrorResponseType};
@@ -382,7 +383,11 @@ impl TokenSet {
             did: did.clone(),
         };
 
-        let nbf = Utc::now().add(chrono::Duration::seconds(access_token_lifetime - 60));
+        let nbf = if *DISABLE_REFRESH_TOKEN_NBF {
+            Utc::now()
+        } else {
+            Utc::now().add(chrono::Duration::seconds(access_token_lifetime - 60))
+        };
         let nbf_unix = UnixTimeStamp::from_secs(nbf.timestamp() as u64);
 
         let claims =


Test patch: