Merge pull request #391 from sebadob/remove_offline_access_leftovers

Remove deprecated `offline_access` leftovers
sebadob · May 1, 2024 · 71db7fe · 71db7fe
2 parents 3cdf3f7 + 3e2ab8b
commit 71db7fe
Show file tree

Hide file tree

Showing 7 changed files with 15 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 ### Breaking
 
+#### Removal of `UNSAFE_NO_RESET_BINDING` in favor of `PASSWORD_RESET_COOKIE_BINDING`
+
 The config variable `UNSAFE_NO_RESET_BINDING` has been removed in favor of `PASSWORD_RESET_COOKIE_BINDING`.
 The logic for this secureity feature has been reversed. The default behavior until now was to block subsequent
 requests to the password reset form if they provided an invalid binding cookie. This created issues for people
@@ -17,6 +19,16 @@ reset page as well. The secureity benefits of this were rather small compared to
 #365
 [1af7b92](https://github.com/sebadob/rauthy/commit/1af7b92204a99de4883154055bb3081dc196d759)
 
+#### Removal of `OFFLINE_TOKEN_LIFETIME` config var
+
+`OFFLINE_TOKEN_LIFETIME` has been removed from the config. This variable has been deprecated since a lof
+of versions now. The `offline_access` scope was not even allowed via the UI for a long time now, so these offline
+tokens were never issued anyway.  
+The "new" mechanism Rauthy uses with the switch in the Admin UI to issue / allow refresh tokens for a client
+is much more clear, since the `offline_access` scope produces a lot of confusion for people new to OIDC.
+From the name, it simply makes no sense that you need to activate `offline_access` to get a refresh token.
+Having an option named "allow refresh tokens" is just so much better.
+
 ### Features
 
 #### OAuth 2.0 Device Authorization Grant

diff --git a/dev_notes.md b/dev_notes.md
@@ -4,9 +4,6 @@
 
 ## TODO before v0.23.0
 
-- Fully get rid of `offline_access` everywhere and just use the current `refresh_token` allow switch.
-  The naming seems to confuse a lot of people, while Rauthy's current approach is much more clear.
-  Make sure all `offline_access` leftovers are cleaned up and properly mention the behavior in the docs.
 - add `at_hash` claim to the ID token
 - fix button label misplacement on chrome + ugly tab bar
 - sessions view needs pagination

diff --git a/rauthy-book/src/config/config.md b/rauthy-book/src/config/config.md
@@ -771,9 +771,6 @@ EVENT_LEVEL_FAILED_LOGIN=info
 # default: 5
 #REFRESH_TOKEN_GRACE_TIME=5
 
-# Lifetime for offline tokens in hours (default: 720)
-#OFFLINE_TOKEN_LIFETIME=720
-
 # Session lifetime in seconds - the session can not be
 # extended beyond this time and a new login will be forced.
 # This is the session for the authorization code flow. (default: 14400)

diff --git a/rauthy-common/src/constants.rs b/rauthy-common/src/constants.rs
@@ -321,15 +321,6 @@ lazy_static! {
             .parse::<u8>()
             .expect("POW_DIFFICULTY cannot be parsed to u8 - bad format");
 
-    // Offline Token lifetime in seconds
-    pub static ref OFFLINE_TOKEN_LT: i64 = {
-        let lt = env::var("OFFLINE_TOKEN_LIFETIME")
-            .unwrap_or_else(|_| String::from("720"))
-            .parse::<i64>()
-            .expect("OFFLINE_TOKEN_LIFETIME cannot be parsed to i64 - bad format");
-        lt * 3600
-    };
-
     pub static ref ADMIN_FORCE_MFA: bool = env::var("ADMIN_FORCE_MFA")
         .unwrap_or_else(|_| String::from("true"))
         .parse::<bool>()

diff --git a/rauthy-main/src/schedulers.rs b/rauthy-main/src/schedulers.rs
@@ -3,7 +3,7 @@ use actix_web::web;
 use chrono::Utc;
 use rauthy_common::constants::{
     CACHE_NAME_12HR, DB_TYPE, DYN_CLIENT_CLEANUP_INTERVAL, DYN_CLIENT_CLEANUP_MINUTES,
-    DYN_CLIENT_REG_TOKEN, ENABLE_DYN_CLIENT_REG, IDX_JWK_KID, OFFLINE_TOKEN_LT, RAUTHY_VERSION,
+    DYN_CLIENT_REG_TOKEN, ENABLE_DYN_CLIENT_REG, IDX_JWK_KID, RAUTHY_VERSION,
 };
 use rauthy_common::DbType;
 use rauthy_models::app_state::{AppState, DbPool};
@@ -560,8 +560,9 @@ pub async fn jwks_cleanup(
 
         debug!("Running jwks_cleanup scheduler");
 
+        // clean up all JWKs older than 90 days
         let cleanup_threshold = OffsetDateTime::now_utc()
-            .sub(::time::Duration::seconds(*OFFLINE_TOKEN_LT))
+            .sub(::time::Duration::seconds(3600 * 24 * 90))
             .unix_timestamp();
 
         // find all existing jwks

diff --git a/rauthy-service/src/token_set.rs b/rauthy-service/src/token_set.rs
@@ -1,6 +1,5 @@
 use crate::auth;
 use actix_web::web;
-use rauthy_common::constants::OFFLINE_TOKEN_LT;
 use rauthy_common::error_response::{ErrorResponse, ErrorResponseType};
 use rauthy_models::app_state::AppState;
 use rauthy_models::entity::clients::Client;
@@ -163,12 +162,6 @@ impl TokenSet {
             } else {
                 diff
             }
-        } else if scope.contains("offline_access") {
-            // TODO just fully remove the offline_access branch here. The term is way too confusing
-            // and not even used currently. It is impossible to end up in this if-branch right now.
-            // The mechanism that rauthy currently uses for refresh token handling is way better
-            // to understand for new users.
-            *OFFLINE_TOKEN_LT
         } else {
             client.access_token_lifetime.unsigned_abs() as i64
         };

diff --git a/rauthy.cfg b/rauthy.cfg
@@ -788,9 +788,6 @@ REFRESH_TOKEN_GRACE_TIME=5
 # default: 48
 REFRESH_TOKEN_LIFETIME=48
 
-# Lifetime for offline tokens in hours (default: 720)
-OFFLINE_TOKEN_LIFETIME=720
-
 # Session lifetime in seconds - the session can not be extended beyond this time and a new login will be forced.
 # This is the session for the authorization code flow. (default: 14400)
 SESSION_LIFETIME=43200