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 74b3369a9472..d185f82228e9 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,9 +45,22 @@ def call(access_token:, refresh_token: nil, known_audiences: [], clear_previous: @user.oidc_user_tokens.destroy_all if clear_previous - token = @user.oidc_user_tokens.build(access_token:, refresh_token:, audiences: Array(known_audiences)) - # We should discover further audiences from the token in the future + token = @user.oidc_user_tokens.build(access_token:, refresh_token:) + token.audiences = merge_audiences(known_audiences, discover_audiences(access_token)) token.save! if token.audiences.any? end + + private + + def discover_audiences(access_token) + decoded, = ProviderTokenParser.new(verify_audience: false, required_claims: ["aud"]).parse(access_token) + Array(decoded["aud"]) + rescue StandardError + [] + end + + def merge_audiences(*args) + args.flatten.uniq + 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 98743ef0d014..4f688f047ad5 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 @@ -37,8 +37,12 @@ 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"] } } + 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 @@ -48,7 +52,7 @@ expect(token.user_id).to eq user.id expect(token.access_token).to eq access_token expect(token.refresh_token).to eq refresh_token - expect(token.audiences).to eq ["io"] + expect(token.audiences).to contain_exactly("io", "aud1", "aud2") end it "logs no error" do @@ -56,6 +60,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("io", "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("io") + 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 } @@ -66,7 +116,7 @@ expect(token.user_id).to eq user.id expect(token.access_token).to eq access_token expect(token.refresh_token).to be_nil - expect(token.audiences).to eq ["io"] + expect(token.audiences).to contain_exactly("io", "aud1", "aud2") end it "logs no error" do @@ -104,14 +154,32 @@ context "when there is no audience" do let(:args) { { access_token:, refresh_token:, known_audiences: [] } } - 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 context "when another user token existed before" do @@ -129,7 +197,7 @@ expect(token.user_id).to eq user.id expect(token.access_token).to eq access_token expect(token.refresh_token).to eq refresh_token - expect(token.audiences).to eq ["io"] + expect(token.audiences).to contain_exactly("io", "aud1", "aud2") end context "and when previous tokens shall be cleared" do @@ -147,7 +215,7 @@ expect(token.user_id).to eq user.id expect(token.access_token).to eq access_token expect(token.refresh_token).to eq refresh_token - expect(token.audiences).to eq ["io"] + expect(token.audiences).to contain_exactly("io", "aud1", "aud2") end it "logs no error" do @@ -156,4 +224,15 @@ end end end + + context "when audiences from token and arguments overlap" do + let(:parsed_jwt) { { "aud" => ["io", "aud2"] } } + + it "normalizes the audience array" do + subject + + token = OpenIDConnect::UserToken.last + expect(token.audiences).to contain_exactly("io", "aud2") + end + end end