From d834a1d4d049e5e640479805f5a0a72ccc14b4eb Mon Sep 17 00:00:00 2001 From: theworldbright Date: Fri, 31 Jul 2015 18:02:59 +0900 Subject: [PATCH] Replace user info endpoint with supported claims The route /api/v0/user/ will now be used as a non-OIDC route. In other words, the /api/v0/user/ will require the "read" scope while /api/openid_connect/user_info/ will require the "openid" scope --- .../openid_connect/discovery_controller.rb | 4 ++-- .../openid_connect/user_info_controller.rb | 19 +++++++++++++++ app/serializers/user_info_serializer.rb | 23 +++++++++++++++++++ app/serializers/user_serializer.rb | 3 --- config/routes.rb | 2 ++ features/step_definitions/auth_code_steps.rb | 2 +- .../step_definitions/implicit_flow_steps.rb | 2 +- .../step_definitions/oidc_common_steps.rb | 10 ++++---- .../authorizations_controller_spec.rb | 6 +++-- spec/integration/api/users_controller_spec.rb | 9 ++++---- .../protected_resource_endpoint_spec.rb | 11 +++++---- 11 files changed, 68 insertions(+), 23 deletions(-) create mode 100644 app/controllers/api/openid_connect/user_info_controller.rb create mode 100644 app/serializers/user_info_serializer.rb delete mode 100644 app/serializers/user_serializer.rb diff --git a/app/controllers/api/openid_connect/discovery_controller.rb b/app/controllers/api/openid_connect/discovery_controller.rb index 4d4b56bb5..15ce38738 100644 --- a/app/controllers/api/openid_connect/discovery_controller.rb +++ b/app/controllers/api/openid_connect/discovery_controller.rb @@ -25,8 +25,8 @@ module Api request_object_signing_alg_values_supported: %i(HS256 HS384 HS512), subject_types_supported: %w(public pairwise), id_token_signing_alg_values_supported: %i(RS256), - token_endpoint_auth_methods_supported: %w(client_secret_basic client_secret_post) - # TODO: claims_supported: ["sub", "iss", "name", "email"] + token_endpoint_auth_methods_supported: %w(client_secret_basic client_secret_post), + claims_supported: %w(sub nickname profile picture zoneinfo) ) end end diff --git a/app/controllers/api/openid_connect/user_info_controller.rb b/app/controllers/api/openid_connect/user_info_controller.rb new file mode 100644 index 000000000..9fb79226d --- /dev/null +++ b/app/controllers/api/openid_connect/user_info_controller.rb @@ -0,0 +1,19 @@ +module Api + module OpenidConnect + class UserInfoController < ApplicationController + include Api::OpenidConnect::ProtectedResourceEndpoint + + before_action do + require_access_token Api::OpenidConnect::Scope.find_by(name: "openid") + end + + def show + render json: current_user, serializer: UserInfoSerializer + end + + def current_user + current_token ? current_token.authorization.user : nil + end + end + end +end diff --git a/app/serializers/user_info_serializer.rb b/app/serializers/user_info_serializer.rb new file mode 100644 index 000000000..3f042380d --- /dev/null +++ b/app/serializers/user_info_serializer.rb @@ -0,0 +1,23 @@ +class UserInfoSerializer < ActiveModel::Serializer + attributes :sub, :nickname, :profile, :picture, :zoneinfo + + def sub + object.diaspora_handle # TODO: Change to proper sub + end + + def nickname + object.name + end + + def profile + File.join(AppConfig.environment.url, "people", object.guid).to_s + end + + def picture + File.join(AppConfig.environment.url, object.image_url).to_s + end + + def zoneinfo + object.language + end +end diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb deleted file mode 100644 index b0d134815..000000000 --- a/app/serializers/user_serializer.rb +++ /dev/null @@ -1,3 +0,0 @@ -class UserSerializer < ActiveModel::Serializer - attributes :name, :email, :language, :username -end diff --git a/config/routes.rb b/config/routes.rb index a9739b73a..683e1da17 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -250,6 +250,8 @@ Diaspora::Application.routes.draw do get ".well-known/webfinger", to: "discovery#webfinger" get ".well-known/openid-configuration", to: "discovery#configuration" get "jwks.json", to: "id_tokens#jwks" + + get "user_info", to: "user_info#show" end end end diff --git a/features/step_definitions/auth_code_steps.rb b/features/step_definitions/auth_code_steps.rb index 8f690a23d..a34c284d9 100644 --- a/features/step_definitions/auth_code_steps.rb +++ b/features/step_definitions/auth_code_steps.rb @@ -28,5 +28,5 @@ end When /^I parse the tokens and use it obtain user info$/ do client_json = JSON.parse(last_response.body) access_token = client_json["access_token"] - get api_v0_user_path, access_token: access_token + get api_openid_connect_user_info_path, access_token: access_token end diff --git a/features/step_definitions/implicit_flow_steps.rb b/features/step_definitions/implicit_flow_steps.rb index fad520faa..cb7394da1 100644 --- a/features/step_definitions/implicit_flow_steps.rb +++ b/features/step_definitions/implicit_flow_steps.rb @@ -33,7 +33,7 @@ end When /^I parse the bearer tokens and use it to access user info$/ do access_token = current_url[/(?<=access_token=)[^&]+/] - get api_v0_user_path, access_token: access_token + get api_openid_connect_user_info_path, access_token: access_token end Then /^I should see an "([^\"]*)" error$/ do |error_message| diff --git a/features/step_definitions/oidc_common_steps.rb b/features/step_definitions/oidc_common_steps.rb index 1519e26fc..bcf8be96e 100644 --- a/features/step_definitions/oidc_common_steps.rb +++ b/features/step_definitions/oidc_common_steps.rb @@ -9,19 +9,19 @@ end When /^I use received valid bearer tokens to access user info$/ do access_token_json = JSON.parse(last_response.body) - get api_v0_user_path, access_token: access_token_json["access_token"] + get api_openid_connect_user_info_path, access_token: access_token_json["access_token"] end When /^I use invalid bearer tokens to access user info$/ do - get api_v0_user_path, access_token: SecureRandom.hex(32) + get api_openid_connect_user_info_path, access_token: SecureRandom.hex(32) end Then /^I should receive "([^\"]*)"'s id, username, and email$/ do |username| user_info_json = JSON.parse(last_response.body) user = User.find_by_username(username) - expect(user_info_json["username"]).to have_content(user.username) - expect(user_info_json["language"]).to have_content(user.language) - expect(user_info_json["email"]).to have_content(user.email) + user_profile_url = File.join(AppConfig.environment.url, "people", user.guid).to_s + expect(user_info_json["profile"]).to have_content(user_profile_url) + expect(user_info_json["zoneinfo"]).to have_content(user.language) end Then /^I should receive an "([^\"]*)" error$/ do |error_message| diff --git a/spec/controllers/api/openid_connect/authorizations_controller_spec.rb b/spec/controllers/api/openid_connect/authorizations_controller_spec.rb index a8d5e3405..6663f281e 100644 --- a/spec/controllers/api/openid_connect/authorizations_controller_spec.rb +++ b/spec/controllers/api/openid_connect/authorizations_controller_spec.rb @@ -94,8 +94,10 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do end end context "when already authorized" do - let!(:auth) { Api::OpenidConnect::Authorization.find_or_create_by(o_auth_application: client, user: alice, - redirect_uri: "http://localhost:3000/") } + let!(:auth) { + Api::OpenidConnect::Authorization.find_or_create_by(o_auth_application: client, user: alice, + redirect_uri: "http://localhost:3000/") + } context "when valid parameters are passed" do before do diff --git a/spec/integration/api/users_controller_spec.rb b/spec/integration/api/users_controller_spec.rb index 53f9b4607..7b13f738f 100644 --- a/spec/integration/api/users_controller_spec.rb +++ b/spec/integration/api/users_controller_spec.rb @@ -8,20 +8,21 @@ describe Api::V0::UsersController do end let(:auth_with_read) do auth = Api::OpenidConnect::Authorization.create!(o_auth_application: client, user: alice) - auth.scopes << [Api::OpenidConnect::Scope.find_or_create_by(name: "read")] + auth.scopes << [Api::OpenidConnect::Scope.find_or_create_by(name: "openid"), + Api::OpenidConnect::Scope.find_or_create_by(name: "read")] auth end let!(:access_token_with_read) { auth_with_read.create_access_token.to_s } describe "#show" do before do - get api_v0_user_path, access_token: access_token_with_read + get api_openid_connect_user_info_path, access_token: access_token_with_read end it "shows the info" do json_body = JSON.parse(response.body) - expect(json_body["username"]).to eq(alice.username) - expect(json_body["email"]).to eq(alice.email) + expect(json_body["nickname"]).to eq(alice.name) + expect(json_body["profile"]).to eq(File.join(AppConfig.environment.url, "people", alice.guid).to_s) end end end diff --git a/spec/lib/api/openid_connect/protected_resource_endpoint_spec.rb b/spec/lib/api/openid_connect/protected_resource_endpoint_spec.rb index f3933a5ee..219cf1c40 100644 --- a/spec/lib/api/openid_connect/protected_resource_endpoint_spec.rb +++ b/spec/lib/api/openid_connect/protected_resource_endpoint_spec.rb @@ -8,7 +8,8 @@ describe Api::OpenidConnect::ProtectedResourceEndpoint, type: :request do end let(:auth_with_read) do auth = Api::OpenidConnect::Authorization.create!(o_auth_application: client, user: alice) - auth.scopes << [Api::OpenidConnect::Scope.find_or_create_by(name: "read")] + auth.scopes << [Api::OpenidConnect::Scope.find_or_create_by(name: "openid"), + Api::OpenidConnect::Scope.find_or_create_by(name: "read")] auth end let!(:access_token_with_read) { auth_with_read.create_access_token.to_s } @@ -18,7 +19,7 @@ describe Api::OpenidConnect::ProtectedResourceEndpoint, type: :request do context "when valid access token is provided" do before do - get api_v0_user_path, access_token: access_token_with_read + get api_openid_connect_user_info_path, access_token: access_token_with_read end it "includes private in the cache-control header" do @@ -28,7 +29,7 @@ describe Api::OpenidConnect::ProtectedResourceEndpoint, type: :request do context "when no access token is provided" do before do - get api_v0_user_path + get api_openid_connect_user_info_path end it "should respond with a 401 Unauthorized response" do @@ -41,7 +42,7 @@ describe Api::OpenidConnect::ProtectedResourceEndpoint, type: :request do context "when an invalid access token is provided" do before do - get api_v0_user_path, access_token: invalid_token + get api_openid_connect_user_info_path, access_token: invalid_token end it "should respond with a 401 Unauthorized response" do @@ -60,7 +61,7 @@ describe Api::OpenidConnect::ProtectedResourceEndpoint, type: :request do context "when authorization has been destroyed" do before do auth_with_read.destroy - get api_v0_user_path, access_token: access_token_with_read + get api_openid_connect_user_info_path, access_token: access_token_with_read end it "should respond with a 401 Unauthorized response" do