From efc65c857665dd7b69449a8dea644c3763871e58 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Mon, 16 Dec 2024 10:12:57 +0100 Subject: [PATCH] Discover audiences from access token We want to know for which purposes tokens can be used. Assuming that we receive JWTs as access tokens, it's possible to read their audience and thus check where these tokens are usable. Importantly, it's still possible that an access token is not a JWT, so we have to allow that as well. The code could be extended in the future to send such tokens to the introspection endpoint of the IDP, hoping to receive an audience list as a result of that. --- .../openid_connect/associate_user_token.rb | 9 ++- .../associate_user_token_spec.rb | 76 ++++++++++++++++++- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/modules/openid_connect/app/services/openid_connect/associate_user_token.rb b/modules/openid_connect/app/services/openid_connect/associate_user_token.rb index 60c4be3dad77..ffdaa5c55dcd 100644 --- a/modules/openid_connect/app/services/openid_connect/associate_user_token.rb +++ b/modules/openid_connect/app/services/openid_connect/associate_user_token.rb @@ -45,7 +45,7 @@ def call(access_token:, refresh_token: nil, assume_idp: false) end token = user_session.oidc_user_tokens.build(access_token:, refresh_token:) - # We should discover further audiences from the token in the future + token.audiences = discover_audiences(access_token) token.audiences << UserToken::IDP_AUDIENCE if assume_idp token.save! if token.audiences.any? @@ -57,5 +57,12 @@ def find_user_session private_session_id = @session.id.private_id ::Sessions::UserSession.find_by(session_id: private_session_id) end + + def discover_audiences(access_token) + decoded, = ProviderTokenParser.new(verify_audience: false, required_claims: ["aud"]).parse(access_token) + Array(decoded["aud"]) + rescue StandardError + [] + end end end diff --git a/modules/openid_connect/spec/services/openid_connect/associate_user_token_spec.rb b/modules/openid_connect/spec/services/openid_connect/associate_user_token_spec.rb index addf7d0d38cd..faa7c280d8cc 100644 --- a/modules/openid_connect/spec/services/openid_connect/associate_user_token_spec.rb +++ b/modules/openid_connect/spec/services/openid_connect/associate_user_token_spec.rb @@ -40,10 +40,14 @@ let(:access_token) { "access-token-foo" } let(:refresh_token) { "refresh-token-bar" } + let(:parser) { instance_double(OpenIDConnect::ProviderTokenParser, parse: [parsed_jwt, nil]) } + let(:parsed_jwt) { { "aud" => ["aud1", "aud2"] } } + let!(:user_session) { create(:user_session, session_id: session.id.private_id) } before do allow(Rails.logger).to receive(:error) + allow(OpenIDConnect::ProviderTokenParser).to receive(:new).and_return(parser) end it "creates a correct user token", :aggregate_failures do @@ -52,7 +56,7 @@ token = OpenIDConnect::UserToken.last expect(token.access_token).to eq access_token expect(token.refresh_token).to eq refresh_token - expect(token.audiences).to eq ["__op-idp__"] + expect(token.audiences).to contain_exactly("__op-idp__", "aud1", "aud2") end it "logs no error" do @@ -60,6 +64,52 @@ expect(Rails.logger).not_to have_received(:error) end + it "correctly tries parsing the access token" do + subject + + expect(OpenIDConnect::ProviderTokenParser).to have_received(:new) + .with(verify_audience: false, required_claims: ["aud"]) + expect(parser).to have_received(:parse).with(access_token) + end + + context "when the JWT encodes aud as a string" do + let(:parsed_jwt) { { "aud" => "aud1" } } + + it "creates a correct user token", :aggregate_failures do + expect { subject }.to change(OpenIDConnect::UserToken, :count).by(1) + + token = OpenIDConnect::UserToken.last + expect(token.access_token).to eq access_token + expect(token.refresh_token).to eq refresh_token + expect(token.audiences).to contain_exactly("__op-idp__", "aud1") + end + + it "logs no error" do + subject + expect(Rails.logger).not_to have_received(:error) + end + end + + context "when the access token is not a valid JWT" do + before do + allow(parser).to receive(:parse).and_raise("Oops, not a JWT!") + end + + it "creates a correct user token", :aggregate_failures do + expect { subject }.to change(OpenIDConnect::UserToken, :count).by(1) + + token = OpenIDConnect::UserToken.last + expect(token.access_token).to eq access_token + expect(token.refresh_token).to eq refresh_token + expect(token.audiences).to contain_exactly("__op-idp__") + end + + it "logs no error" do + subject + expect(Rails.logger).not_to have_received(:error) + end + end + context "when there is no refresh token" do let(:refresh_token) { nil } @@ -69,7 +119,7 @@ token = OpenIDConnect::UserToken.last expect(token.access_token).to eq access_token expect(token.refresh_token).to be_nil - expect(token.audiences).to eq ["__op-idp__"] + expect(token.audiences).to contain_exactly("__op-idp__", "aud1", "aud2") end it "logs no error" do @@ -107,13 +157,31 @@ context "when we are not allowed to assume the token has the IDP audience" do let(:args) { { access_token:, refresh_token: } } - it "does not create a user token" do - expect { subject }.not_to change(OpenIDConnect::UserToken, :count) + it "creates a correct user token", :aggregate_failures do + expect { subject }.to change(OpenIDConnect::UserToken, :count).by(1) + + token = OpenIDConnect::UserToken.last + expect(token.access_token).to eq access_token + expect(token.refresh_token).to eq refresh_token + expect(token.audiences).to contain_exactly("aud1", "aud2") end it "logs no error" do subject expect(Rails.logger).not_to have_received(:error) end + + context "and the token has no audience defined" do + let(:parsed_jwt) { { "sub" => "ject" } } + + it "does not create a user token" do + expect { subject }.not_to change(OpenIDConnect::UserToken, :count) + end + + it "logs no error" do + subject + expect(Rails.logger).not_to have_received(:error) + end + end end end