Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Comment on lines +19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the impression that refresh_tokens usually are opaque (which seems to be supported by at least one of the standards), so I am not sure nbf-checking of refresh-tokens usually would be related to a projects quality 😅

AT expiry though, should probably best be checked (at least for clients like oauth2-proxy)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. exp checking on the access_token must be done though and I don't see any good reason why you would refresh a still valid access_token, other than simply being lazy in the implementation.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the refresh token just another JWT to give clients the possibility to inspect it and gain further information. Even though it's non-standard, it's an additional feature.

# 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
Expand Down
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions book/src/config/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions rauthy.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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: ''
Expand Down
1 change: 1 addition & 0 deletions src/api/src/oidc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,7 @@ pub async fn post_token(
Ok(resp)
}
Err(err) => {
error!("{}", err.message);
if !has_password_been_hashed {
return Err(err);
}
Expand Down
4 changes: 4 additions & 0 deletions src/common/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
2 changes: 1 addition & 1 deletion src/common/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
9 changes: 7 additions & 2 deletions src/service/src/token_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 =
Expand Down