From b65d415af4283f06a037aa4bec09b3b7c7553d1c Mon Sep 17 00:00:00 2001
From: James Mead <james.mead@digital.cabinet-office.gov.uk>
Date: Fri, 6 Oct 2023 09:44:54 +0100
Subject: [PATCH 1/2] Small improvement to SSOPushCredentialTest

Since we have a reference to the authorisation created in the setup, it
seems odd not to use it.
---
 test/lib/sso_push_credential_test.rb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/test/lib/sso_push_credential_test.rb b/test/lib/sso_push_credential_test.rb
index 869ccdec4..47daf52f9 100644
--- a/test/lib/sso_push_credential_test.rb
+++ b/test/lib/sso_push_credential_test.rb
@@ -8,13 +8,12 @@ class SSOPushCredentialTest < ActiveSupport::TestCase
 
   context "given an already authorised application" do
     setup do
-      authorisation = @user.authorisations.create!(application_id: @application.id)
-      authorisation.update!(token: "foo")
+      @authorisation = @user.authorisations.create!(application_id: @application.id)
     end
 
     should "return the bearer token for an already-authorized application" do
       bearer_token = SSOPushCredential.credentials(@application)
-      assert_equal "foo", bearer_token
+      assert_equal @authorisation.token, bearer_token
     end
 
     should "create required application permissions if they do not already exist" do

From 0a718b943b92cea881c8ab908a1afdf82cc57ba4 Mon Sep 17 00:00:00 2001
From: James Mead <james.mead@digital.cabinet-office.gov.uk>
Date: Thu, 5 Oct 2023 15:30:11 +0100
Subject: [PATCH 2/2] Ignore expired & revoked tokens in
 SSOPushCredential.credentials

Trello: https://trello.com/c/piKwrxYP

Previously when a token reached its (10 year) expiry, the code in
SSOPushCredential.credentials was still finding the expired token and
trying to use it in the SSOPushClient, but triggering a SSOPushError
exception due to an invalid bearer token.

While we were debugging this also realised that
SSOPushCredential.credentials was also finding revoked tokens, thus
making it impossible to resolve the situation from the Signon UI.

Now we ignore both expired & revoked tokens when deciding whether we
already have a valid token or that we need to create one.

Note that the `Doorkeeper::AccessToken.not_expired` scope [1] seems to
helpfully exclude both expired *and* revoked tokens.

[1]: https://github.com/doorkeeper-gem/doorkeeper/blob/986115cc228ff30dc1ead0f4101195448994f5d4/lib/doorkeeper/orm/active_record/mixins/access_token.rb#L47-L66
---
 lib/sso_push_credential.rb           |  1 +
 test/lib/sso_push_credential_test.rb | 30 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/lib/sso_push_credential.rb b/lib/sso_push_credential.rb
index 5e6452e84..38093a4fa 100644
--- a/lib/sso_push_credential.rb
+++ b/lib/sso_push_credential.rb
@@ -9,6 +9,7 @@ def credentials(application)
       user.grant_application_permissions(application, PERMISSIONS)
 
       user.authorisations
+        .not_expired
         .create_with(expires_in: 10.years)
         .find_or_create_by!(application_id: application.id).token
     end
diff --git a/test/lib/sso_push_credential_test.rb b/test/lib/sso_push_credential_test.rb
index 47daf52f9..217d9b2f3 100644
--- a/test/lib/sso_push_credential_test.rb
+++ b/test/lib/sso_push_credential_test.rb
@@ -37,6 +37,36 @@ class SSOPushCredentialTest < ActiveSupport::TestCase
     end
   end
 
+  context "given an application with a revoked authorisation" do
+    setup do
+      @user.authorisations.create!(application_id: @application.id, revoked_at: Time.current)
+    end
+
+    should "create a new authorisation to replace the revoked one" do
+      bearer_token = SSOPushCredential.credentials(@application)
+
+      new_authorisation = @user.authorisations.find_by(token: bearer_token)
+      assert_nil new_authorisation.revoked_at
+      assert_equal @application.id, new_authorisation.application_id
+    end
+  end
+
+  context "given an application with an expired authorisation" do
+    setup do
+      travel(-1.day) do
+        @user.authorisations.create!(application_id: @application.id, expires_in: 0)
+      end
+    end
+
+    should "create a new authorisation to replace the expired one" do
+      bearer_token = SSOPushCredential.credentials(@application)
+
+      new_authorisation = @user.authorisations.find_by(token: bearer_token)
+      assert new_authorisation.expires_at > Time.current
+      assert_equal @application.id, new_authorisation.application_id
+    end
+  end
+
   should "create an authorisation if one does not already exist" do
     assert_equal 0, @user.authorisations.count