From 0935451cd80cf6020540a62030b09dac9e49ba14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Fri, 24 Jan 2020 11:02:02 +0100 Subject: [PATCH 1/3] Return a default token_endpoint_auth_method when the client gives none in its OpenID Connect registration request Since we announce it in the supported metadata, some clients expect to be told what to use and don't fallback to the spec standard of client_secret_basic on their own. --- .../api/openid_connect/o_auth_application.rb | 3 ++- .../openid_connect/clients_controller_spec.rb | 26 ++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/app/models/api/openid_connect/o_auth_application.rb b/app/models/api/openid_connect/o_auth_application.rb index d29856601..8d4005c39 100644 --- a/app/models/api/openid_connect/o_auth_application.rb +++ b/app/models/api/openid_connect/o_auth_application.rb @@ -46,7 +46,8 @@ module Api def as_json(opts={}) data = super - data[:client_secret_expires_at] = 0 + data["client_secret_expires_at"] = 0 + data["token_endpoint_auth_method"] ||= "client_secret_post" data end diff --git a/spec/controllers/api/openid_connect/clients_controller_spec.rb b/spec/controllers/api/openid_connect/clients_controller_spec.rb index 63beab027..ce51fee21 100644 --- a/spec/controllers/api/openid_connect/clients_controller_spec.rb +++ b/spec/controllers/api/openid_connect/clients_controller_spec.rb @@ -3,7 +3,7 @@ describe Api::OpenidConnect::ClientsController, type: :controller, suppress_csrf_verification: :none do describe "#create" do context "when valid parameters are passed" do - it "should return a client id" do + before do stub_request(:get, "http://example.com/uris") .with(headers: { "Accept" => "*/*", @@ -15,15 +15,27 @@ describe Api::OpenidConnect::ClientsController, type: :controller, suppress_csrf response_types: [], grant_types: [], application_type: "web", contacts: [], logo_uri: "http://example.com/logo.png", client_uri: "http://example.com/client", policy_uri: "http://example.com/policy", tos_uri: "http://example.com/tos", - sector_identifier_uri: "http://example.com/uris", subject_type: "pairwise"} + sector_identifier_uri: "http://example.com/uris", subject_type: "pairwise"} end + + it "should return a client id" do client_json = JSON.parse(response.body) expect(client_json["client_id"].length).to eq(32) expect(client_json["ppid"]).to eq(true) end + + it "should return a client secret expiration time" do + client_json = JSON.parse(response.body) + expect(client_json["client_secret_expires_at"]).to eq(0) + end + + it "should return a default token endpoint authentication method" do + client_json = JSON.parse(response.body) + expect(client_json["token_endpoint_auth_method"]).to eq("client_secret_post") + end end context "when valid parameters with jwks is passed" do - it "should return a client id" do + before do stub_request(:get, "http://example.com/uris") .with(headers: { "Accept" => "*/*", @@ -77,10 +89,18 @@ describe Api::OpenidConnect::ClientsController, type: :controller, suppress_csrf } ] }} + end + + it "should return a client id" do client_json = JSON.parse(response.body) expect(client_json["client_id"].length).to eq(32) expect(client_json["ppid"]).to eq(true) end + + it "should retain the token endpoint authentication method" do + client_json = JSON.parse(response.body) + expect(client_json["token_endpoint_auth_method"]).to eq("private_key_jwt") + end end context "when valid parameters with jwks_uri is passed" do From 35bfbc9c828d3a2de65e308623ad3252f394c6bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Fri, 24 Jan 2020 16:58:32 +0100 Subject: [PATCH 2/3] Return missing created_at field on reshares endpoint --- app/controllers/api/v1/reshares_controller.rb | 1 + lib/schemas/api_v1.json | 14 ++++++++++++++ spec/integration/api/reshares_controller_spec.rb | 2 +- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/reshares_controller.rb b/app/controllers/api/v1/reshares_controller.rb index 48f9edf81..8caca9e7e 100644 --- a/app/controllers/api/v1/reshares_controller.rb +++ b/app/controllers/api/v1/reshares_controller.rb @@ -25,6 +25,7 @@ module Api reshares_page[:data] = reshares_page[:data].map do |r| { guid: r.guid, + created_at: r.created_at, author: PersonPresenter.new(r.author).as_api_json } end diff --git a/lib/schemas/api_v1.json b/lib/schemas/api_v1.json index 30006d6ef..6faa4c04f 100644 --- a/lib/schemas/api_v1.json +++ b/lib/schemas/api_v1.json @@ -337,6 +337,20 @@ ] }, + "reshares": { + "type": "array", + "items": { + "type": "object", + "properties": { + "guid": { "$ref": "#/definitions/guid" }, + "created_at": { "$ref": "#/definitions/timestamp" }, + "author": { "$ref": "#/definitions/short_profile" } + }, + "required": ["guid", "created_at", "author"], + "additionalProperties": false + } + }, + "posts": { "type": "array", "items": { "$ref": "#/definitions/post" } diff --git a/spec/integration/api/reshares_controller_spec.rb b/spec/integration/api/reshares_controller_spec.rb index 5f3cd7c79..328426238 100644 --- a/spec/integration/api/reshares_controller_spec.rb +++ b/spec/integration/api/reshares_controller_spec.rb @@ -55,7 +55,7 @@ describe Api::V1::ResharesController do expect(reshare["guid"]).not_to be_nil confirm_person_format(reshare["author"], alice) - expect(reshares.to_json).to match_json_schema(:api_v1_schema) + expect(reshares.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/reshares") end it "succeeds but empty with private post it can see" do From 654b81b8f138006d7d5d6a18cbfc1dd794232d37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Fri, 24 Jan 2020 16:59:04 +0100 Subject: [PATCH 3/3] Explicitly select fragment from API JSON schema in specs this should make them more strict and robust at the same time --- spec/integration/api/aspects_controller_spec.rb | 4 ++-- spec/integration/api/comments_controller_spec.rb | 2 +- spec/integration/api/contacts_controller_spec.rb | 2 +- .../api/conversations_controller_spec.rb | 4 ++-- spec/integration/api/likes_controller_spec.rb | 2 +- spec/integration/api/messages_controller_spec.rb | 2 +- .../api/notifications_controller_spec.rb | 4 ++-- spec/integration/api/photos_controller_spec.rb | 4 ++-- spec/integration/api/posts_controller_spec.rb | 6 +++--- spec/integration/api/search_controller_spec.rb | 8 ++++---- spec/integration/api/streams_controller_spec.rb | 14 +++++++------- .../api/tag_followings_controller_spec.rb | 2 +- spec/integration/api/users_controller_spec.rb | 14 +++++++------- 13 files changed, 34 insertions(+), 34 deletions(-) diff --git a/spec/integration/api/aspects_controller_spec.rb b/spec/integration/api/aspects_controller_spec.rb index e96125efb..9a9621cf1 100644 --- a/spec/integration/api/aspects_controller_spec.rb +++ b/spec/integration/api/aspects_controller_spec.rb @@ -46,7 +46,7 @@ describe Api::V1::AspectsController do expect(aspect["order"]).to eq(found_aspect.order_id) end - expect(aspects.to_json).to match_json_schema(:api_v1_schema) + expect(aspects.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/aspects") end context "without impromper credentials" do @@ -81,7 +81,7 @@ describe Api::V1::AspectsController do expect(aspect["name"]).to eq(@aspect2.name) expect(aspect["order"]).to eq(@aspect2.order_id) - expect(aspect.to_json).to match_json_schema(:api_v1_schema) + expect(aspect.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/aspect") end end diff --git a/spec/integration/api/comments_controller_spec.rb b/spec/integration/api/comments_controller_spec.rb index 60eea9816..ae6a5cf81 100644 --- a/spec/integration/api/comments_controller_spec.rb +++ b/spec/integration/api/comments_controller_spec.rb @@ -144,7 +144,7 @@ describe Api::V1::CommentsController do confirm_comment_format(comments[0], auth.user, @comment_text1) confirm_comment_format(comments[1], auth.user, @comment_text2) - expect(comments.to_json).to match_json_schema(:api_v1_schema) + expect(comments.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/comments_or_messages") end end diff --git a/spec/integration/api/contacts_controller_spec.rb b/spec/integration/api/contacts_controller_spec.rb index faabb41d0..5e5b76c1f 100644 --- a/spec/integration/api/contacts_controller_spec.rb +++ b/spec/integration/api/contacts_controller_spec.rb @@ -58,7 +58,7 @@ describe Api::V1::ContactsController do contacts = response_body_data(response) expect(contacts.length).to eq(@aspect1.contacts.length) - expect(contacts.to_json).to match_json_schema(:api_v1_schema) + expect(contacts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/users") end end diff --git a/spec/integration/api/conversations_controller_spec.rb b/spec/integration/api/conversations_controller_spec.rb index c88d53cda..4d903b4ab 100644 --- a/spec/integration/api/conversations_controller_spec.rb +++ b/spec/integration/api/conversations_controller_spec.rb @@ -170,7 +170,7 @@ describe Api::V1::ConversationsController do actual_conversation = returned_conversations.select {|c| c["guid"] == @read_conversation_guid }[0] confirm_conversation_format(actual_conversation, @read_conversation, [auth.user, alice]) - expect(returned_conversations.to_json).to match_json_schema(:api_v1_schema) + expect(returned_conversations.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/conversations") end it "returns all the user unread conversations" do @@ -227,7 +227,7 @@ describe Api::V1::ConversationsController do conversation = response_body(response) confirm_conversation_format(conversation, @conversation, [auth.user, alice]) - expect(conversation.to_json).to match_json_schema(:api_v1_schema) + expect(conversation.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/conversation") end end diff --git a/spec/integration/api/likes_controller_spec.rb b/spec/integration/api/likes_controller_spec.rb index b462ef27b..d25d82265 100644 --- a/spec/integration/api/likes_controller_spec.rb +++ b/spec/integration/api/likes_controller_spec.rb @@ -75,7 +75,7 @@ describe Api::V1::LikesController do confirm_like_format(likes, bob) confirm_like_format(likes, auth.user) - expect(likes.to_json).to match_json_schema(:api_v1_schema) + expect(likes.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/likes") end end diff --git a/spec/integration/api/messages_controller_spec.rb b/spec/integration/api/messages_controller_spec.rb index 3fc365146..d2956b017 100644 --- a/spec/integration/api/messages_controller_spec.rb +++ b/spec/integration/api/messages_controller_spec.rb @@ -131,7 +131,7 @@ describe Api::V1::MessagesController do messages = response_body_data(response) expect(messages.length).to eq(1) - expect(messages.to_json).to match_json_schema(:api_v1_schema) + expect(messages.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/comments_or_messages") confirm_message_format(messages[0], "first message", auth.user) conversation = get_conversation(@conversation_guid) diff --git a/spec/integration/api/notifications_controller_spec.rb b/spec/integration/api/notifications_controller_spec.rb index 6c685537c..0edbd857e 100644 --- a/spec/integration/api/notifications_controller_spec.rb +++ b/spec/integration/api/notifications_controller_spec.rb @@ -41,7 +41,7 @@ describe Api::V1::NotificationsController do expect(notifications.length).to eq(2) confirm_notification_format(notifications[1], @notification, "also_commented", nil) - expect(notifications.to_json).to match_json_schema(:api_v1_schema) + expect(notifications.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/notifications") end it "with proper credentials and unread only" do @@ -120,7 +120,7 @@ describe Api::V1::NotificationsController do notification = JSON.parse(response.body) confirm_notification_format(notification, @notification, "also_commented", @post) - expect(notification.to_json).to match_json_schema(:api_v1_schema) + expect(notification.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/notification") end end diff --git a/spec/integration/api/photos_controller_spec.rb b/spec/integration/api/photos_controller_spec.rb index 48f5837c7..457d67db9 100644 --- a/spec/integration/api/photos_controller_spec.rb +++ b/spec/integration/api/photos_controller_spec.rb @@ -78,7 +78,7 @@ describe Api::V1::PhotosController do expect(photo.has_key?("post")).to be_falsey confirm_photo_format(photo, @user_photo1) - expect(photo.to_json).to match_json_schema(:api_v1_schema) + expect(photo.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/photo") end it "with correct GUID user's photo used in post and access token" do @@ -152,7 +152,7 @@ describe Api::V1::PhotosController do photos = response_body_data(response) expect(photos.length).to eq(2) - expect(photos.to_json).to match_json_schema(:api_v1_schema) + expect(photos.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/photos") end end diff --git a/spec/integration/api/posts_controller_spec.rb b/spec/integration/api/posts_controller_spec.rb index c2bb045f3..6cecda8b8 100644 --- a/spec/integration/api/posts_controller_spec.rb +++ b/spec/integration/api/posts_controller_spec.rb @@ -74,7 +74,7 @@ describe Api::V1::PostsController do post = response_body(response) confirm_post_format(post, alice, @status, [bob, eve]) - expect(post.to_json).to match_json_schema(:api_v1_schema) + expect(post.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/post") end end @@ -98,7 +98,7 @@ describe Api::V1::PostsController do post = response_body(response) confirm_post_format(post, alice, status_message) - expect(post.to_json).to match_json_schema(:api_v1_schema) + expect(post.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/post") end end @@ -115,7 +115,7 @@ describe Api::V1::PostsController do post = response_body(response) confirm_reshare_format(post, @status, alice) - expect(post.to_json).to match_json_schema(:api_v1_schema) + expect(post.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/post") end end diff --git a/spec/integration/api/search_controller_spec.rb b/spec/integration/api/search_controller_spec.rb index cc22f457a..73d1b084e 100644 --- a/spec/integration/api/search_controller_spec.rb +++ b/spec/integration/api/search_controller_spec.rb @@ -63,7 +63,7 @@ describe Api::V1::SearchController do users = response_body_data(response) expect(users.length).to eq(15) - expect(users.to_json).to match_json_schema(:api_v1_schema) + expect(users.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/users") end it "succeeds by name" do @@ -75,7 +75,7 @@ describe Api::V1::SearchController do users = response_body_data(response) expect(users.length).to eq(1) - expect(users.to_json).to match_json_schema(:api_v1_schema) + expect(users.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/users") end it "succeeds by handle" do @@ -87,7 +87,7 @@ describe Api::V1::SearchController do users = response_body_data(response) expect(users.length).to eq(1) - expect(users.to_json).to match_json_schema(:api_v1_schema) + expect(users.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/users") end it "doesn't return closed accounts" do @@ -184,7 +184,7 @@ describe Api::V1::SearchController do posts = response_body_data(response) expect(posts.length).to eq(2) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "only returns public posts without private scope" do diff --git a/spec/integration/api/streams_controller_spec.rb b/spec/integration/api/streams_controller_spec.rb index 5a88b53fb..ea80d1539 100644 --- a/spec/integration/api/streams_controller_spec.rb +++ b/spec/integration/api/streams_controller_spec.rb @@ -84,7 +84,7 @@ describe Api::V1::StreamsController do posts = response_body_data(response) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "contains expected aspect message" do @@ -146,7 +146,7 @@ describe Api::V1::StreamsController do posts = response_body_data(response) expect(posts.length).to eq(3) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "public posts only tags expected" do @@ -185,7 +185,7 @@ describe Api::V1::StreamsController do expect(response.status).to eq(200) posts = response_body_data(response) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "contains activity message" do @@ -228,7 +228,7 @@ describe Api::V1::StreamsController do expect(response.status).to eq(200) posts = response_body_data(response) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "contains main message" do @@ -271,7 +271,7 @@ describe Api::V1::StreamsController do expect(response.status).to eq(200) posts = response_body_data(response) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "contains commented message" do @@ -312,7 +312,7 @@ describe Api::V1::StreamsController do expect(response.status).to eq(200) posts = response_body_data(response) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "contains mentions message" do @@ -353,7 +353,7 @@ describe Api::V1::StreamsController do expect(response.status).to eq(200) posts = response_body_data(response) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "contains liked message" do diff --git a/spec/integration/api/tag_followings_controller_spec.rb b/spec/integration/api/tag_followings_controller_spec.rb index bcb066db5..76ccc1588 100755 --- a/spec/integration/api/tag_followings_controller_spec.rb +++ b/spec/integration/api/tag_followings_controller_spec.rb @@ -104,7 +104,7 @@ describe Api::V1::TagFollowingsController do expect(items.length).to eq(@expected_tags.length) @expected_tags.each {|tag| expect(items.find(tag)).to be_truthy } - expect(items.to_json).to match_json_schema(:api_v1_schema) + expect(items.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/tags") end end diff --git a/spec/integration/api/users_controller_spec.rb b/spec/integration/api/users_controller_spec.rb index 226900c13..d13d0af24 100644 --- a/spec/integration/api/users_controller_spec.rb +++ b/spec/integration/api/users_controller_spec.rb @@ -64,7 +64,7 @@ describe Api::V1::UsersController do expect(user["guid"]).to eq(auth.user.guid) confirm_self_data_format(user) - expect(user.to_json).to match_json_schema(:api_v1_schema) + expect(user.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/own_user") end it "fails if invalid token" do @@ -89,7 +89,7 @@ describe Api::V1::UsersController do expect(user["guid"]).to eq(alice.person.guid) confirm_public_profile_hash(user) - expect(user.to_json).to match_json_schema(:api_v1_schema) + expect(user.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/user") end it "succeeds with in Aspect valid user" do @@ -106,7 +106,7 @@ describe Api::V1::UsersController do expect(user["guid"]).to eq(alice.person.guid) confirm_public_profile_hash(user) - expect(user.to_json).to match_json_schema(:api_v1_schema) + expect(user.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/user") end it "succeeds with limited data on non-public/not shared" do @@ -133,7 +133,7 @@ describe Api::V1::UsersController do expect(user["guid"]).to eq(eve.person.guid) confirm_public_profile_hash(user) - expect(user.to_json).to match_json_schema(:api_v1_schema) + expect(user.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/user") end it "fails if invalid token" do @@ -327,7 +327,7 @@ describe Api::V1::UsersController do expect(contacts.length).to eq(1) confirm_person_format(contacts[0], alice) - expect(contacts.to_json).to match_json_schema(:api_v1_schema) + expect(contacts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/users") end it "fails with invalid GUID" do @@ -395,7 +395,7 @@ describe Api::V1::UsersController do expect(guids).not_to include(@private_photo1.guid) confirm_photos(photos) - expect(photos.to_json).to match_json_schema(:api_v1_schema) + expect(photos.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/photos") end it "returns only public photos of other user without private:read scope in token" do @@ -472,7 +472,7 @@ describe Api::V1::UsersController do post = posts.select {|p| p["guid"] == @public_post1.guid } confirm_post_format(post[0], alice, @public_post1) - expect(posts.to_json).to match_json_schema(:api_v1_schema) + expect(posts.to_json).to match_json_schema(:api_v1_schema, fragment: "#/definitions/posts") end it "returns logged in user's posts" do