From 8364a5ddc46d677ba95bfeef33ddbd02660298f6 Mon Sep 17 00:00:00 2001 From: sdi Date: Sat, 1 Aug 2026 15:10:03 +0300 Subject: [PATCH 1/5] add oauth oidc --- .gitignore | 3 + Gemfile | 5 + Gemfile.lock | 7 +- app/admin/system/oauth_applications.rb | 130 ++++++++++++++++++ .../oauth_authorization_server_controller.rb | 17 +++ app/models/oauth_access_token.rb | 12 ++ app/models/oauth_openid_request.rb | 25 ++++ app/policies/oauth_application_policy.rb | 20 +++ config/initializers/config.rb | 11 ++ config/initializers/doorkeeper.rb | 21 ++- .../initializers/doorkeeper_openid_connect.rb | 117 ++++++++++++++++ config/locales/en.yml | 9 ++ config/policy_roles.yml_ | 3 + config/routes.rb | 20 ++- config/yeti_web.yml.ci | 7 + config/yeti_web.yml.development | 7 + config/yeti_web.yml.distr | 16 +++ ...create_doorkeeper_openid_connect_tables.rb | 24 ++++ db/structure.sql | 61 ++++++++ lib/tasks/oauth.rake | 35 +++++ .../system/oauth_applications_spec.rb | 111 +++++++++++++++ spec/fixtures/oidc_test_signing_key.pem | 28 ++++ .../policies/oauth_application_policy_spec.rb | 94 +++++++++++++ .../oauth/openid_configuration_spec.rb | 48 +++++++ .../oauth/openid_connect_flow_spec.rb | 125 +++++++++++++++++ spec/requests/oauth/userinfo_spec.rb | 38 +++++ spec/requests/oauth/well_known_spec.rb | 22 +++ 27 files changed, 1012 insertions(+), 4 deletions(-) create mode 100644 app/admin/system/oauth_applications.rb create mode 100644 app/models/oauth_openid_request.rb create mode 100644 app/policies/oauth_application_policy.rb create mode 100644 config/initializers/doorkeeper_openid_connect.rb create mode 100644 db/migrate/20260729120000_create_doorkeeper_openid_connect_tables.rb create mode 100644 lib/tasks/oauth.rake create mode 100644 spec/features/system/oauth_applications_spec.rb create mode 100644 spec/fixtures/oidc_test_signing_key.pem create mode 100644 spec/policies/oauth_application_policy_spec.rb create mode 100644 spec/requests/oauth/openid_configuration_spec.rb create mode 100644 spec/requests/oauth/openid_connect_flow_spec.rb create mode 100644 spec/requests/oauth/userinfo_spec.rb diff --git a/.gitignore b/.gitignore index c2151a243..fe1be71f8 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,9 @@ config/click_house.yml config/secrets.yml config/ldap.yml config/oidc.yml +# Signs the id_tokens yeti issues as an OIDC provider — reading it is enough to +# forge an identity for any admin. Deploy it out of band. +config/oidc_signing_key.pem coverage debian/files debian/yeti-web diff --git a/Gemfile b/Gemfile index 10d2b96d5..ffecba100 100644 --- a/Gemfile +++ b/Gemfile @@ -17,6 +17,11 @@ gem 'responders' gem 'activeadmin-oidc', github: 'activeadmin-plugins/activeadmin-oidc' gem 'devise', '>= 4.6.0' gem 'doorkeeper', '~> 5.9' +# OIDC layer on top of Doorkeeper: id_token, /.well-known/openid-configuration, +# JWKS and userinfo. Always configured (it has to be — see +# config/initializers/doorkeeper_openid_connect.rb), but exposes nothing unless +# YetiConfig.oauth.oidc.enabled. +gem 'doorkeeper-openid_connect', '~> 1.10' gem 'ostruct', '~> 0.6.3' # Seamless JWT authentication for Rails API diff --git a/Gemfile.lock b/Gemfile.lock index e39f11e9c..6329f1258 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -317,6 +317,10 @@ GEM docile (1.4.1) doorkeeper (5.9.3) railties (>= 5) + doorkeeper-openid_connect (1.10.5) + doorkeeper (>= 5.5, < 6.0) + jwt (>= 2.5) + ostruct (>= 0.5) draper (4.0.6) actionpack (>= 5.0) activemodel (>= 5.0) @@ -457,7 +461,7 @@ GEM csv mini_mime (>= 1.0.0) multi_xml (>= 0.5.2) - httpx (1.8.0) + httpx (1.8.1) http-2 (>= 1.2.0) i18n (1.15.2) concurrent-ruby (~> 1.0) @@ -1106,6 +1110,7 @@ DEPENDENCIES delayed_job_active_record devise (>= 4.6.0) doorkeeper (~> 5.9) + doorkeeper-openid_connect (~> 1.10) draper dry-validation (~> 1.0) elasticsearch diff --git a/app/admin/system/oauth_applications.rb b/app/admin/system/oauth_applications.rb new file mode 100644 index 000000000..76f1e0b94 --- /dev/null +++ b/app/admin/system/oauth_applications.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +# Registered OAuth/OIDC clients — the things that sign users in through yeti or +# call its API: yeti-statistics, Grafana, an internal tool. This page is the only +# way to register one. +# +# MCP clients (Claude Code, Cursor, ...) do not need this page: they self-register +# through POST /oauth/register (RFC 7591) as public PKCE clients. They still show +# up in the list once they have. +# +# Access is role-gated by OauthApplicationPolicy, and root-only until some role +# is granted the "System/OauthApplication" section in config/policy_roles.yml. +# That default matters more here than on most pages: the show page displays the +# client secret in cleartext — which is how Doorkeeper stores it, and which an +# operator configuring a client has to be able to read back. +ActiveAdmin.register OauthApplication do + menu parent: ['System', 'Admin Access'], label: 'OAuth Applications', priority: 98 + + config.batch_actions = false + config.sort_order = 'created_at_desc' + + # uid and secret may only be chosen at registration time — letting them change + # afterwards would silently break a client that is already using them, and the + # form doesn't offer them on edit. Permitting them only on create means a + # hand-crafted POST can't do it either. + permit_params do + permitted = %i[name redirect_uri confidential] + permitted += %i[uid secret] if params[:action] == 'create' + permitted + [{ scopes: [] }] + end + + filter :name + filter :uid, label: 'Client ID' + filter :scopes + filter :confidential + filter :created_at + + index do + column :id + column :name + column 'Client ID', :uid + column :scopes + column :confidential + column :redirect_uri + column :created_at + actions + end + + show do + attributes_table do + row :id + row :name + row('Client ID', &:uid) + # Cleartext, deliberately: this is the only place an operator can recover + # it, and Doorkeeper is storing it in cleartext regardless. + row('Client secret', &:plaintext_secret) + row :scopes + row('Confidential', &:confidential?) + row :redirect_uri + row :created_at + row :updated_at + end + + panel 'Active tokens' do + # Deleting the client deletes these with it (dependent: :delete_all). + para "#{oauth_application.access_tokens.where(revoked_at: nil).count} not revoked" + end + end + + form do |f| + f.semantic_errors(*f.object.errors.attribute_names) + + f.inputs do + f.input :name, hint: 'Shown in the consent screen the admin is asked to approve.' + f.input :redirect_uri, + as: :text, + input_html: { rows: 3 }, + hint: 'Where the client is sent back after sign-in. Must match what the client ' \ + 'sends byte for byte, base path included, and must be HTTPS unless the host ' \ + 'is a loopback address. One per line for several.' + # Before :scopes, not after — a lone boolean checkbox rendered directly + # beneath the scopes checkbox list reads as one more scope. + f.input :confidential, + hint: 'On for a client that can keep a secret (a server, like yeti-statistics). ' \ + 'Off for a public client that authenticates with PKCE alone.' + f.input :scopes, + as: :check_boxes, + collection: Doorkeeper.config.scopes.all, + hint: 'openid is what makes this an OIDC login and produces an id_token. ' \ + 'Leave empty to grant the default scopes.' + + if f.object.new_record? + # required: false — the model does validate presence, but Doorkeeper + # fills both in before validation when they are blank, so marking them + # required would claim the operator has to invent them. + f.input :uid, label: 'Client ID', + required: false, + hint: 'Leave blank to generate. Set it to a fixed value when the client ' \ + 'config is deployed from a template.' + f.input :secret, label: 'Client secret', + required: false, + hint: 'Leave blank to generate.' + end + end + + f.actions + end + + # Replaces a leaked or rotated secret without deleting the client, so existing + # tokens keep working — only the client's ability to get new ones is affected + # until its config is updated. + member_action :rotate_secret, method: :put do + resource.renew_secret + resource.save! + redirect_to resource_path(resource), notice: 'Client secret rotated. The client must be reconfigured with the new one.' + end + + action_item :rotate_secret, only: :show do + if authorized?(:rotate_secret) + # Url options and html options must stay separate hashes here, or `method` + # and `data` end up as query parameters and the link silently becomes a GET + # with no confirmation. + link_to 'Rotate secret', + { action: :rotate_secret, id: resource.id }, + method: :put, + data: { confirm: 'Replace this client secret? The client stops being able to ' \ + 'obtain new tokens until it is reconfigured.' } + end + end +end diff --git a/app/controllers/well_known/oauth_authorization_server_controller.rb b/app/controllers/well_known/oauth_authorization_server_controller.rb index b59ec642b..877b3f43d 100644 --- a/app/controllers/well_known/oauth_authorization_server_controller.rb +++ b/app/controllers/well_known/oauth_authorization_server_controller.rb @@ -19,6 +19,23 @@ def show code_challenge_methods_supported: %w[S256], token_endpoint_auth_methods_supported: %w[none client_secret_basic], service_documentation: "#{request.base_url}/api/mcp" + }.merge(oidc_metadata) + end + + private + + # When yeti is also an OIDC provider, say so here. A client that reads only + # this document (it is served at a path the OIDC gem would otherwise claim + # — see config/routes.rb) should not conclude there is no OIDC on offer. + # The authoritative OIDC document is /.well-known/openid-configuration. + def oidc_metadata + return {} unless YetiConfig.oauth&.oidc&.enabled + + { + jwks_uri: "#{request.base_url}/oauth/discovery/keys", + userinfo_endpoint: "#{request.base_url}/oauth/userinfo", + id_token_signing_alg_values_supported: %w[RS256], + subject_types_supported: %w[public] } end end diff --git a/app/models/oauth_access_token.rb b/app/models/oauth_access_token.rb index a24ca81f8..b4698051b 100644 --- a/app/models/oauth_access_token.rb +++ b/app/models/oauth_access_token.rb @@ -37,4 +37,16 @@ class OauthAccessToken < ApplicationRecord # the AA index page can eager-load with `.includes(:resource_owner)` and # avoid an N+1 in the Owner column for root admins. belongs_to :resource_owner, class_name: 'AdminUser', foreign_key: :resource_owner_id, optional: true + + # A token is only as valid as the admin behind it. Doorkeeper's own answer is + # "not expired and not revoked", which leaves a disabled admin's unexpired + # token working until it runs out — up to an hour of access after offboarding, + # on every endpoint that authorizes with it (/oauth/userinfo, introspection, + # /api/mcp). Checking the owner here revokes that access immediately, in one + # place, instead of once per endpoint. + def accessible? + return super if resource_owner_id.nil? + + super && AdminUser.exists?(id: resource_owner_id, enabled: true) + end end diff --git a/app/models/oauth_openid_request.rb b/app/models/oauth_openid_request.rb new file mode 100644 index 000000000..0e0341c48 --- /dev/null +++ b/app/models/oauth_openid_request.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: gui.oauth_openid_requests +# Database name: primary +# +# id :bigint(8) not null, primary key +# nonce :string not null +# access_grant_id :bigint(8) not null +# +# Indexes +# +# index_oauth_openid_requests_on_access_grant_id (access_grant_id) +# +# Foreign Keys +# +# oauth_openid_requests_access_grant_id_fkey (access_grant_id => oauth_access_grants.id) ON DELETE => cascade +# +class OauthOpenidRequest < ApplicationRecord + include ::Doorkeeper::OpenidConnect::Orm::ActiveRecord::Mixins::OpenidRequest + # The mixin assigns table_name itself, so this override has to come after the + # include — same as the other gui-schema OAuth models. + self.table_name = 'gui.oauth_openid_requests' +end diff --git a/app/policies/oauth_application_policy.rb b/app/policies/oauth_application_policy.rb new file mode 100644 index 000000000..fe9aad70f --- /dev/null +++ b/app/policies/oauth_application_policy.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# Policy for the AA "OAuth Applications" page — the registered OAuth/OIDC +# clients. Page-level access is governed by role config like any other admin +# page (config/policy_roles.yml, section "System/OauthApplication"): `read` +# controls who sees the page, `change` who can register or edit a client, +# `remove` who can delete one, and `perform` who can rotate a client secret. +# +# `read` is more sensitive here than on most pages: the show page displays the +# client secret, because Doorkeeper stores it in cleartext (hash_application_secrets +# is off) and an operator wiring up a client needs to read it back. +class OauthApplicationPolicy < ::RolePolicy + alias_rule :rotate_secret?, to: :perform? + + private + + def section_name + :'System/OauthApplication' + end +end diff --git a/config/initializers/config.rb b/config/initializers/config.rb index f9dab3f86..31d35cebd 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -96,6 +96,17 @@ def self.setting_files(config_root, _env) # Block AND `enabled` key are both optional; missing → treated as false. optional(:oauth).schema do optional(:enabled).value(:bool?) + + # Turns the OAuth provider into an OIDC provider as well: id_token, + # /.well-known/openid-configuration, JWKS and userinfo. Requires + # oauth.enabled. `issuer` and `signing_key_path` are mandatory once + # enabled, but that is enforced in the initializer rather than here so + # the message can say what to do about it. + optional(:oidc).schema do + optional(:enabled).value(:bool?) + optional(:issuer).maybe(:string) + optional(:signing_key_path).maybe(:string) + end end # Mounts /api/mcp. Requires oauth.enabled (MCP authenticates via OAuth diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 67afe6672..3c1007bb6 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -37,8 +37,10 @@ ) end - # Who can manage OAuth applications via Doorkeeper's built-in /oauth/applications - # web UI? Block all access — admins manage tokens via the AA page instead. + # Who can manage OAuth applications via Doorkeeper's built-in + # /oauth/applications web UI? Nobody — clients are managed through the + # ActiveAdmin "OAuth Applications" page instead, and routes.rb no longer + # mounts that controller at all. Kept as a backstop in case it ever is. admin_authenticator do |_routes| head 403 end @@ -200,6 +202,13 @@ # MCP clients (Claude Code, Cursor) are public clients and MUST use PKCE. force_pkce + # S256 only. Doorkeeper also accepts `plain`, where the verifier travels in + # the authorization request in the clear and PKCE protects nothing — while + # /.well-known/oauth-authorization-server has always advertised S256 alone. + # This makes that claim true, and keeps it consistent with the OIDC + # discovery document, which reports this option verbatim. + pkce_code_challenge_methods ['S256'] + # Store SHA256 of tokens, not plaintext. Stolen DB dump can't be used to # impersonate users. hash_token_secrets @@ -262,6 +271,14 @@ # default_scopes :mcp + # OIDC scopes, offered only when oauth.oidc.enabled — withholding `openid` + # is what keeps the OIDC layer dormant, since it is the scope that asks for + # an id_token and nothing else can trigger one. Deliberately not a default + # scope either: a client must request it explicitly. See + # config/initializers/doorkeeper_openid_connect.rb for the claims each one + # unlocks. + optional_scopes :openid, :profile, :email if YetiConfig.oauth.oidc&.enabled + # Allows to restrict only certain scopes for grant_type. # By default, all the scopes will be available for all the grant types. # diff --git a/config/initializers/doorkeeper_openid_connect.rb b/config/initializers/doorkeeper_openid_connect.rb new file mode 100644 index 000000000..cd2773a0e --- /dev/null +++ b/config/initializers/doorkeeper_openid_connect.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +# OpenID Connect layer on top of the Doorkeeper OAuth provider configured in +# doorkeeper.rb (which must run first — this file's name sorts after it, and +# Doorkeeper::OpenidConnect.configure reads Doorkeeper's ORM setting). +# +# OAuth 2 answers "may this client call the API?"; OIDC answers "who is the +# user?". Enabling this makes admin_users the login for other yeti components — +# yeti-statistics, Grafana, anything that speaks OIDC — by issuing a signed +# id_token alongside the access token. +# +# Unlike doorkeeper.rb, this block is NOT skipped when the feature is off. The +# gem prepends an `openid_request` association onto every Doorkeeper access +# grant model, and that association reads +# Doorkeeper::OpenidConnect.configuration while the class body is evaluated — +# so an unconfigured gem makes OauthAccessGrant raise MissingConfiguration the +# moment anything loads it, which in production means at eager load, in every +# deployment, whether or not OAuth is even enabled. +# +# So configuration always happens, and oauth.oidc.enabled decides only whether +# the OIDC surface is exposed: the `openid` scope (doorkeeper.rb) and the +# discovery / JWKS / userinfo routes (routes.rb). With no `openid` scope on +# offer, no client can be granted one, no id_token is ever minted, and the +# signing key below is never read — which is why it may be absent when the +# feature is off. +oidc_enabled = YetiConfig.oauth&.enabled && YetiConfig.oauth.oidc&.enabled +oidc_issuer = nil +oidc_signing_key = nil + +if oidc_enabled + oidc_issuer = YetiConfig.oauth.oidc.issuer.presence + if oidc_issuer.nil? + raise 'yeti_web.yml: oauth.oidc.issuer is required when oauth.oidc.enabled. ' \ + 'It must equal, byte for byte, the issuer configured on every client — ' \ + 'clients compare it against the discovery document and the id_token, ' \ + 'and a trailing slash is enough to break every login.' + end + + key_path = YetiConfig.oauth.oidc.signing_key_path.presence + if key_path.nil? + raise 'yeti_web.yml: oauth.oidc.signing_key_path is required when oauth.oidc.enabled. ' \ + 'Generate one with: bundle exec rake oauth:oidc:generate_signing_key[/etc/yeti-web/oidc_signing_key.pem]' + end + + key_path = Rails.root.join(key_path) unless Pathname.new(key_path).absolute? + unless File.readable?(key_path) + raise "yeti_web.yml: oauth.oidc.signing_key_path #{key_path} is missing or unreadable. " \ + "Generate one with: bundle exec rake oauth:oidc:generate_signing_key[#{key_path}]" + end + oidc_signing_key = File.read(key_path) +end + +Doorkeeper::OpenidConnect.configure do + issuer oidc_issuer + signing_key oidc_signing_key + signing_algorithm :rs256 + subject_types_supported [:public] + + # Custom model so the table lives in the gui schema, like the other + # Doorkeeper tables. See app/models/oauth_openid_request.rb. + open_id_request_class 'OauthOpenidRequest' + + # `sub` identifies the user forever and must never be recycled. The primary + # key qualifies; the email does not — an admin who changes address would + # come back to every client as a different person. + subject { |resource_owner, _application| resource_owner.id.to_s } + + # Filtering on enabled here is what stops a disabled admin's still-valid + # access token from resolving at the userinfo endpoint. Token issuance is + # blocked separately by the before_successful_strategy_response hook in + # doorkeeper.rb. + resource_owner_from_access_token do |access_token| + AdminUser.find_by(id: access_token.resource_owner_id, enabled: true) + end + + auth_time_from_resource_owner(&:current_sign_in_at) + + # Honours prompt=login: drop the Devise session and send the admin back + # through the normal sign-in page, then on to where they were going. + reauthenticate_resource_owner do |_resource_owner, return_to| + store_location_for :admin_user, return_to + sign_out :admin_user + redirect_to new_admin_user_session_url + end + + # Claim generators are called with (resource_owner, scopes, access_token). + # + # `response:` decides which document a claim appears in, and it defaults to + # [:user_info] alone. Clients that read the id_token and never call + # /oauth/userinfo — the common case, and what yeti-statistics does — would + # otherwise get a token carrying nothing but `sub`, sign the user in as + # anonymous, and then fail whatever group check they run. So every claim is + # declared for both. + claims do + claim(:name, response: %i[id_token user_info]) { |resource_owner, _scopes| resource_owner.display_name } + claim(:preferred_username, response: %i[id_token user_info]) { |resource_owner, _scopes| resource_owner.username } + + # AdminUser#email is not a column — it is read off the billing contact, so + # it is nil for any admin who has none. Emitting nil is the honest answer; + # clients that need an address fall back to `preferred_username`, which is + # the username and always present. + claim(:email, response: %i[id_token user_info]) { |resource_owner, _scopes| resource_owner.email } + # Set by another admin on the billing contact, never self-asserted — so an + # address that exists is as verified as this provider can make it. + claim(:email_verified, response: %i[id_token user_info]) { |resource_owner, _scopes| resource_owner.email.present? } + + # The authorisation boundary. Clients match their own allowlist against + # this (yeti-statistics' `allowed_groups`); without it they fall back to + # "anyone this provider knows", which here means every enabled admin can + # read every customer's traffic and margin. + # + # Tied to `openid` rather than the default scope for a non-standard claim + # name, which would be `profile` — a client requesting `openid` alone would + # then silently lose its only means of authorising anyone. + claim(:groups, response: %i[id_token user_info], scope: :openid) { |resource_owner, _scopes| resource_owner.roles } + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index f1764b18a..751291e5f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -180,6 +180,15 @@ en: display_tag_action_value: 'Tag Action Value' activerecord: + models: + # Otherwise ActiveAdmin humanizes the class name to "Oauth Application" + # in the page title, the New button and the pagination counter. + oauth_application: + one: 'OAuth Application' + other: 'OAuth Applications' + oauth_access_token: + one: 'OAuth Access Token' + other: 'OAuth Access Tokens' attributes: customers_auth: ip: 'IP' diff --git a/config/policy_roles.yml_ b/config/policy_roles.yml_ index 56e1c4fdf..9eb5bcb89 100644 --- a/config/policy_roles.yml_ +++ b/config/policy_roles.yml_ @@ -58,6 +58,9 @@ user: change: true remove: true perform: true + Billing/NotificationTemplate: + read: true + change: true Billing/ServiceType: read: true change: true diff --git a/config/routes.rb b/config/routes.rb index 71f7c5a1c..11e251584 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,9 +23,27 @@ def dasherized_resource(name, options = {}, &block) # Doorkeeper OAuth provider + RFC 8414 metadata + RFC 7591 dynamic client # registration. Gated on YetiConfig.oauth.enabled so the surface is opt-in. if YetiConfig.oauth&.enabled - use_doorkeeper + # skip_controllers :applications — Doorkeeper's built-in client-management UI + # is deliberately unavailable (doorkeeper.rb's admin_authenticator answers + # 403), and clients are managed through the ActiveAdmin "OAuth Applications" + # page instead. Leaving it mounted would also claim the oauth_application(s) + # route helper names, which that AA page needs for its own links. + use_doorkeeper do + skip_controllers :applications + end + # Must stay above use_doorkeeper_openid_connect: the OIDC gem's discovery + # routes claim /.well-known/oauth-authorization-server too, and its version + # of that document has no registration_endpoint (it only advertises one for + # its own dynamic registration, which we don't use). First route wins, so + # this ordering is what keeps MCP clients able to self-register. get '/.well-known/oauth-authorization-server', to: 'well_known/oauth_authorization_server#show' post '/oauth/register', to: 'oauth/registrations#create' + + # /.well-known/openid-configuration, /oauth/discovery/keys (JWKS) and + # /oauth/userinfo. Guarded on the same condition as the initializer — + # this helper reads Doorkeeper::OpenidConnect.configuration while routes + # are drawn and raises MissingConfiguration if it was never configured. + use_doorkeeper_openid_connect if YetiConfig.oauth.oidc&.enabled end # MCP server for LLM tool use. Auth via Doorkeeper OAuth bearer tokens, so diff --git a/config/yeti_web.yml.ci b/config/yeti_web.yml.ci index 1b5ab20a0..b44e14f10 100644 --- a/config/yeti_web.yml.ci +++ b/config/yeti_web.yml.ci @@ -43,6 +43,13 @@ sentry: oauth: enabled: true # mount Doorkeeper OAuth provider — required for the spec suite + # OIDC provider — required for the spec suite. The key below is a throwaway + # committed for the tests; it signs nothing real. + oidc: + enabled: true + issuer: http://www.example.com + signing_key_path: spec/fixtures/oidc_test_signing_key.pem + mcp: enabled: true # mount /api/mcp; requires oauth.enabled — required for the spec suite diff --git a/config/yeti_web.yml.development b/config/yeti_web.yml.development index b75a9581e..b34ceecea 100644 --- a/config/yeti_web.yml.development +++ b/config/yeti_web.yml.development @@ -43,6 +43,13 @@ sentry: oauth: enabled: false # mount Doorkeeper OAuth provider (/oauth/*, /.well-known/oauth-authorization-server) + # OpenID Connect provider on top of it — see yeti_web.yml.distr for what each + # key means. `rake oauth:oidc:generate_signing_key` writes the key. + #oidc: + # enabled: true + # issuer: http://127.0.0.1:3000 + # signing_key_path: config/oidc_signing_key.pem + mcp: enabled: false # mount /api/mcp; requires oauth.enabled diff --git a/config/yeti_web.yml.distr b/config/yeti_web.yml.distr index 4548c82c4..363ca1045 100644 --- a/config/yeti_web.yml.distr +++ b/config/yeti_web.yml.distr @@ -43,6 +43,22 @@ sentry: oauth: enabled: false # mount Doorkeeper OAuth provider (/oauth/*, /.well-known/oauth-authorization-server) + # Also act as an OpenID Connect provider, so other yeti components + # (yeti-statistics, Grafana, ...) can use admin_users as their login. Adds + # /.well-known/openid-configuration, /oauth/discovery/keys and + # /oauth/userinfo, and makes the token endpoint issue an id_token for + # clients that request the `openid` scope. + #oidc: + # enabled: true + # # MUST equal, byte for byte, the issuer configured on the client side — + # # a trailing slash is enough to break every login. + # issuer: https://web.example.com + # # RS256 private key that signs the id_token. Generate with + # # bundle exec rake oauth:oidc:generate_signing_key[/etc/yeti-web/oidc_signing_key.pem] + # # and deploy it out of band: whoever can read it can mint an identity for + # # any admin. + # signing_key_path: /etc/yeti-web/oidc_signing_key.pem + mcp: enabled: false # mount /api/mcp; requires oauth.enabled diff --git a/db/migrate/20260729120000_create_doorkeeper_openid_connect_tables.rb b/db/migrate/20260729120000_create_doorkeeper_openid_connect_tables.rb new file mode 100644 index 000000000..5ce64b9ce --- /dev/null +++ b/db/migrate/20260729120000_create_doorkeeper_openid_connect_tables.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# Storage for the OIDC `nonce`, which has to survive between the authorization +# request (where the client sends it) and the token exchange (where it must be +# echoed into the id_token). Without this table every login fails on the +# client's replay check. +class CreateDoorkeeperOpenidConnectTables < ActiveRecord::Migration[7.2] + def up + execute %q{ + CREATE TABLE gui.oauth_openid_requests ( + id bigserial PRIMARY KEY, + access_grant_id bigint NOT NULL REFERENCES gui.oauth_access_grants(id) ON DELETE CASCADE, + nonce varchar NOT NULL + ); + CREATE INDEX index_oauth_openid_requests_on_access_grant_id ON gui.oauth_openid_requests (access_grant_id); + } + end + + def down + execute %q{ + DROP TABLE gui.oauth_openid_requests; + } + end +end diff --git a/db/structure.sql b/db/structure.sql index fb8d8be4e..684f1a31f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14665,6 +14665,36 @@ CREATE SEQUENCE gui.oauth_applications_id_seq ALTER SEQUENCE gui.oauth_applications_id_seq OWNED BY gui.oauth_applications.id; +-- +-- Name: oauth_openid_requests; Type: TABLE; Schema: gui; Owner: - +-- + +CREATE TABLE gui.oauth_openid_requests ( + id bigint NOT NULL, + access_grant_id bigint NOT NULL, + nonce character varying NOT NULL +); + + +-- +-- Name: oauth_openid_requests_id_seq; Type: SEQUENCE; Schema: gui; Owner: - +-- + +CREATE SEQUENCE gui.oauth_openid_requests_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: oauth_openid_requests_id_seq; Type: SEQUENCE OWNED BY; Schema: gui; Owner: - +-- + +ALTER SEQUENCE gui.oauth_openid_requests_id_seq OWNED BY gui.oauth_openid_requests.id; + + -- -- Name: sessions; Type: TABLE; Schema: gui; Owner: - -- @@ -16716,6 +16746,13 @@ ALTER TABLE ONLY gui.oauth_access_tokens ALTER COLUMN id SET DEFAULT nextval('gu ALTER TABLE ONLY gui.oauth_applications ALTER COLUMN id SET DEFAULT nextval('gui.oauth_applications_id_seq'::regclass); +-- +-- Name: oauth_openid_requests id; Type: DEFAULT; Schema: gui; Owner: - +-- + +ALTER TABLE ONLY gui.oauth_openid_requests ALTER COLUMN id SET DEFAULT nextval('gui.oauth_openid_requests_id_seq'::regclass); + + -- -- Name: sessions id; Type: DEFAULT; Schema: gui; Owner: - -- @@ -18184,6 +18221,14 @@ ALTER TABLE ONLY gui.oauth_applications ADD CONSTRAINT oauth_applications_pkey PRIMARY KEY (id); +-- +-- Name: oauth_openid_requests oauth_openid_requests_pkey; Type: CONSTRAINT; Schema: gui; Owner: - +-- + +ALTER TABLE ONLY gui.oauth_openid_requests + ADD CONSTRAINT oauth_openid_requests_pkey PRIMARY KEY (id); + + -- -- Name: sessions sessions_pkey; Type: CONSTRAINT; Schema: gui; Owner: - -- @@ -19340,6 +19385,13 @@ CREATE UNIQUE INDEX index_oauth_access_tokens_on_token ON gui.oauth_access_token CREATE UNIQUE INDEX index_oauth_applications_on_uid ON gui.oauth_applications USING btree (uid); +-- +-- Name: index_oauth_openid_requests_on_access_grant_id; Type: INDEX; Schema: gui; Owner: - +-- + +CREATE INDEX index_oauth_openid_requests_on_access_grant_id ON gui.oauth_openid_requests USING btree (access_grant_id); + + -- -- Name: index_versions_on_item_type_and_item_id; Type: INDEX; Schema: gui; Owner: - -- @@ -20495,6 +20547,14 @@ ALTER TABLE ONLY gui.oauth_access_tokens ADD CONSTRAINT oauth_access_tokens_resource_owner_id_fkey FOREIGN KEY (resource_owner_id) REFERENCES gui.admin_users(id) ON DELETE CASCADE; +-- +-- Name: oauth_openid_requests oauth_openid_requests_access_grant_id_fkey; Type: FK CONSTRAINT; Schema: gui; Owner: - +-- + +ALTER TABLE ONLY gui.oauth_openid_requests + ADD CONSTRAINT oauth_openid_requests_access_grant_id_fkey FOREIGN KEY (access_grant_id) REFERENCES gui.oauth_access_grants(id) ON DELETE CASCADE; + + -- -- Name: contacts contacts_admin_user_id_fkey; Type: FK CONSTRAINT; Schema: notifications; Owner: - -- @@ -20734,6 +20794,7 @@ ALTER TABLE ONLY sys.sensors SET search_path TO gui, public, switch, billing, class4, runtime_stats, sys, logs, data_import; INSERT INTO "public"."schema_migrations" (version) VALUES +('20260729120000'), ('20260719120000'), ('20260705140000'), ('20260704115624'), diff --git a/lib/tasks/oauth.rake b/lib/tasks/oauth.rake new file mode 100644 index 000000000..f88b38a0f --- /dev/null +++ b/lib/tasks/oauth.rake @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +namespace :oauth do + # Clients are registered in the admin UI (System → Admin Access → OAuth + # Applications), not from here. What is left is the one thing the UI cannot + # do: put a private key on the server's filesystem. + namespace :oidc do + # The id_token is signed with this key, so whoever can read it can mint an + # identity for any admin on any client that trusts this issuer. Treat it + # like the database password: 0400, deployed out of band, never in git. + # + # Rotation: publish the new key in JWKS before signing with it, keep the old + # one published until every issued token has expired, then drop the old one. + # + # Usage: + # rake 'oauth:oidc:generate_signing_key[/etc/yeti-web/oidc_signing_key.pem]' + desc 'Generate the RSA key that signs id_tokens' + task :generate_signing_key, [:path] do |_t, args| + path = args[:path].presence || 'config/oidc_signing_key.pem' + raise ArgumentError, "refusing to overwrite existing key at #{path}" if File.exist?(path) + + key = OpenSSL::PKey::RSA.new(2048) + File.write(path, key.to_pem) + File.chmod(0o400, path) + + puts "Wrote a 2048-bit RSA private key to #{path} (mode 0400)." + puts 'Point yeti_web.yml at it:' + puts ' oauth:' + puts ' oidc:' + puts ' enabled: true' + puts ' issuer: https://web.example.com' + puts " signing_key_path: #{path}" + end + end +end diff --git a/spec/features/system/oauth_applications_spec.rb b/spec/features/system/oauth_applications_spec.rb new file mode 100644 index 000000000..3663c37af --- /dev/null +++ b/spec/features/system/oauth_applications_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +# Covers the AA "OAuth Applications" page: registering a client, reading its +# credentials back, rotating the secret, deletion, and role gating. +RSpec.describe 'OAuth Applications page', :js do + include_context :with_oauth_routes + + let!(:application) do + create_oauth_application( + name: 'yeti-statistics', + confidential: true, + scopes: 'openid profile email', + redirect_uri: 'https://stats.example.com/api/auth/callback' + ) + end + + context 'as an admin whose role allows access' do + include_context :login_as_admin + + it 'lists registered clients with their client id' do + visit oauth_applications_path + expect(page).to have_content('yeti-statistics') + expect(page).to have_content(application.uid) + end + + # The whole point of the page: Doorkeeper stores the secret in cleartext and + # an operator configuring a client has to read it back. + it 'shows the client secret on the detail page' do + visit oauth_application_path(application) + expect(page).to have_content(application.uid) + expect(page).to have_content(application.plaintext_secret) + end + + it 'registers a new client with a generated client id and secret' do + visit new_oauth_application_path + fill_in 'Name', with: 'grafana' + fill_in 'Redirect uri', with: 'https://grafana.example.com/login/generic_oauth' + check 'openid' + click_button 'Create' + + created = OauthApplication.find_by(name: 'grafana') + expect(created).to be_present + expect(created.uid).to be_present + expect(created.plaintext_secret).to be_present + expect(created.scopes.to_a).to include('openid') + end + + # Deployments that template the client config from one variable need to pin + # the credentials rather than copy generated ones back out. + it 'accepts a chosen client id and secret' do + visit new_oauth_application_path + fill_in 'Name', with: 'templated-client' + fill_in 'Redirect uri', with: 'https://tpl.example.com/callback' + fill_in 'Client ID', with: 'my-fixed-client-id' + fill_in 'Client secret', with: 'my-fixed-client-secret' + check 'openid' + click_button 'Create' + + created = OauthApplication.find_by(name: 'templated-client') + expect(created.uid).to eq('my-fixed-client-id') + expect(created.plaintext_secret).to eq('my-fixed-client-secret') + end + + it 'rejects a non-loopback redirect uri that is not HTTPS' do + visit new_oauth_application_path + fill_in 'Name', with: 'insecure' + fill_in 'Redirect uri', with: 'http://stats.example.com/callback' + click_button 'Create' + + expect(OauthApplication.find_by(name: 'insecure')).to be_nil + expect(page).to have_content(/redirect uri/i) + end + + it 'rotates the client secret, leaving the client id alone' do + old_secret = application.plaintext_secret + visit oauth_application_path(application) + accept_confirm { click_link 'Rotate secret' } + + expect(page).to have_content('Client secret rotated') + application.reload + expect(application.plaintext_secret).not_to eq(old_secret) + expect(application.uid).to eq(application.reload.uid) + end + + # The uid is not offered on edit — changing it would break a client that is + # already configured with it. + it 'does not offer the client id for editing' do + visit edit_oauth_application_path(application) + expect(page).to have_field('Name') + expect(page).to have_no_field('Client ID') + expect(page).to have_no_field('Client secret') + end + end + + context 'as an admin whose role denies access' do + include_context :login_as_admin + + before do + policy_roles = Rails.configuration.policy_roles.deep_merge( + user: { :'System/OauthApplication' => { read: false } } + ) + allow(Rails.configuration).to receive(:policy_roles).and_return(policy_roles) + end + + it 'is redirected away from the page, so secrets stay hidden' do + visit oauth_applications_path + expect(page).to have_current_path(root_path) + expect(page).not_to have_content(application.plaintext_secret) + end + end +end diff --git a/spec/fixtures/oidc_test_signing_key.pem b/spec/fixtures/oidc_test_signing_key.pem new file mode 100644 index 000000000..524f59f61 --- /dev/null +++ b/spec/fixtures/oidc_test_signing_key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCsKcSZQw2Bcqw1 +NIHqImgv2b1fGAAxOnSrGi0aTHJPQFpbisvAkE5Q2PJWIj1OvqDWVEQFVKS7cEb9 +Acx/LvdOH3Ri2E32AYze63hvFgMZbJ+yk5XtXjT3TC6qREyF5Z3fXA9hd4VVzrEq +UmyoVznNyT6xfH/0oLiA3UonvYvs848bDfrO7pSTqskde/YXO0+CctHcDMUFjlfI +T7IjIduBAzqZq/5sRNOGlkR3xzNP4iXMbQ5Fi2dRhi6OFAaDiHnrIssRV80Y6pTk +pfXcJsLiMppdOOmLTFnGIn+kkWO4q3GYkHbHSeIYtULApRbJVh/p5ul1RTXo2G4U +CVuNSV1PAgMBAAECggEAEfjsQpma29QWQRmhpEkCOliPKdGuF3WEP2ZkpfWsCzTr +GdpT6d7Gpi73oWFCqClDTgyO60WKStzuNDRPXkmPXJLxCe6NuOwxggXv1Rzlpu5N +f41jndtzSQulZXXqzSGKyQnpuyGIhEwm94WSPUKZ0K89Abc+/lEW4bD4MEzbwhkA +7sCw1jHzV1Gah/OqJNydgQHi/aGlDkqJ+2y9xL/1bvZfMEnrhpsdNhNfIgz6xB3x +ziWjr1AcibPVm+wpnWPsZc1SMbUj1WNmyslVB9NUYoF5504It8//8Y55qzdqeVRS +cKrqBh/iFvP/9NTDt7VuxnpIkCcIYc2kDPg6N5CIRQKBgQDkeUgqlbVKr4UW/dAH +aFtaVhqFRxlAx26TNqU80KRtNg9CnoyYgzo5jtG3o1Pr3K+9ZNwXydOe1y14Ogiz +dmsL8syNVJx3xHf5WSVTqA2vr1FttMkuloTaDDduip9CgnFUVvxIHPy7WcDwzJQO +SbWDB9T3YumUY+OgiHr8+BWNmwKBgQDA57jrVm/hk5Y89rWjkGgEYRv/vettWEa1 +4i655G7eO9fXxrHq/Src+kWP3RvIR2i+IJqRJ71Ad8nC2pxDq5AgNKEA/KYr6ASq +KynUPr+xTIQ/nmRRFfFcYdZwz6qiWm/uVWD3tuVv+v1EPuoUGW5qjuxujCIsWU52 +rGsFQOOEXQKBgFIu5iybSWkiFcedaPUhLqsiCcwjNQw+MKI3p9xmWJ8IMRkPmxdJ +LOqDWyCpWYigC38fBqRv7vVWBX8XsQWM9RiJL9cutlHGlDlXPbwH2VR2xd2miC9/ +6S3d8xqKNptf/JAP8hOmiaqCsIptrFwvW2FUseCC7e54t+qI7WSNgIM9AoGAQlev +LiilA7RpSAAlCeKhE1h+c57Sd+GTN/xYy4+3XXDBU7E/AS0eb68W+1o0stV0PWcj +IWbEXIaV+hCLC27We2z+LkO1toXuDMQZd6g47vX8yZBkucpNHtN7F4MkEP//XL06 +QxdAoxlzvuPQRydT5QvQXliNbCkupfu4Nq9RT/UCgYATYc7TMND7KEsUotq5l5nA +/7+qm0Mm96WOZqpjiBRCs6QhMP277D84keneqGnI1HTD0iUI0qAyZMgNpRfaA5qB +Wvj7hNHz+PCRtjnVoS9guefZIvKwtT0LpTJBCnHMFW9lN8w0HC1lqjLXX3mHPsjv +NELohGk68Fl73NjHGk3VEg== +-----END PRIVATE KEY----- diff --git a/spec/policies/oauth_application_policy_spec.rb b/spec/policies/oauth_application_policy_spec.rb new file mode 100644 index 000000000..82f7bfac0 --- /dev/null +++ b/spec/policies/oauth_application_policy_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +RSpec.describe OauthApplicationPolicy do + let(:admin_user) { FactoryBot.build(:admin_user, roles: user_roles) } + let(:policy) { described_class.new(admin_user, OauthApplication.new) } + + before do + allow(Rails.configuration).to receive(:policy_roles).and_return(policy_roles_config) + end + + # Page-level access is role-based (config/policy_roles.yml, section + # "System/OauthApplication"): `read` gates seeing the page — and therefore + # every client secret on it — `change` gates registering and editing, + # `remove` gates deletion, `perform` gates rotating a secret. + describe '#read?' do + context 'when AdminUser is root' do + let(:user_roles) { [:root] } + let(:policy_roles_config) { {} } + + it { expect(policy.read?).to be true } + end + + context 'when the role allows read in the section' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { :'System/OauthApplication' => { read: true } } } } + + it { expect(policy.read?).to be true } + end + + context 'when the role disallows read in the section' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { :'System/OauthApplication' => { read: false } } } } + + it { expect(policy.read?).to be false } + end + + context 'when the section is absent (falls back to the Default section)' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { Default: { read: true } } } } + + it { expect(policy.read?).to be true } + end + end + + describe '#create?' do + context 'when the role allows change' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { :'System/OauthApplication' => { change: true } } } } + + it { expect(policy.create?).to be true } + end + + context 'when the role only allows read' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { :'System/OauthApplication' => { read: true, change: false } } } } + + it { expect(policy.create?).to be false } + end + end + + describe '#destroy?' do + context 'when the role allows remove' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { :'System/OauthApplication' => { remove: true } } } } + + it { expect(policy.destroy?).to be true } + end + + context 'when the role disallows remove' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { :'System/OauthApplication' => { remove: false } } } } + + it { expect(policy.destroy?).to be false } + end + end + + # Rotating a secret is neither editing a field nor deleting the client, so it + # rides on `perform` — the same slot other custom AA actions use. + describe '#rotate_secret?' do + context 'when the role allows perform' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { :'System/OauthApplication' => { perform: true } } } } + + it { expect(policy.rotate_secret?).to be true } + end + + context 'when the role allows change but not perform' do + let(:user_roles) { [:user] } + let(:policy_roles_config) { { user: { :'System/OauthApplication' => { change: true, perform: false } } } } + + it { expect(policy.rotate_secret?).to be false } + end + end +end diff --git a/spec/requests/oauth/openid_configuration_spec.rb b/spec/requests/oauth/openid_configuration_spec.rb new file mode 100644 index 000000000..6c943e2e0 --- /dev/null +++ b/spec/requests/oauth/openid_configuration_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +RSpec.describe 'OIDC discovery', type: :request do + include_context :with_oauth_routes + + describe 'GET /.well-known/openid-configuration' do + subject { get '/.well-known/openid-configuration' } + + let(:payload) { JSON.parse(response.body) } + + it 'advertises the endpoints and algorithms a client needs' do + subject + expect(response).to have_http_status(:success) + + # A client compares this against the URL it was configured with and + # against the id_token's iss; all three must be identical. + expect(payload['issuer']).to eq(YetiConfig.oauth.oidc.issuer) + + expect(payload['authorization_endpoint']).to end_with('/oauth/authorize') + expect(payload['token_endpoint']).to end_with('/oauth/token') + expect(payload['jwks_uri']).to end_with('/oauth/discovery/keys') + expect(payload['userinfo_endpoint']).to end_with('/oauth/userinfo') + + expect(payload['id_token_signing_alg_values_supported']).to include('RS256') + expect(payload['response_types_supported']).to include('code') + expect(payload['code_challenge_methods_supported']).to include('S256') + expect(payload['scopes_supported']).to include('openid') + expect(payload['subject_types_supported']).to include('public') + end + end + + describe 'GET /oauth/discovery/keys' do + subject { get '/oauth/discovery/keys' } + + it 'publishes a public signing key' do + subject + expect(response).to have_http_status(:success) + + keys = JSON.parse(response.body)['keys'] + expect(keys.size).to eq(1) + expect(keys.first).to include('kty' => 'RSA', 'alg' => 'RS256', 'use' => 'sig') + # The kid is how a client picks the right key across a rotation. + expect(keys.first['kid']).to be_present + # Public half only — the private key must never leave the server. + expect(keys.first).not_to have_key('d') + end + end +end diff --git a/spec/requests/oauth/openid_connect_flow_spec.rb b/spec/requests/oauth/openid_connect_flow_spec.rb new file mode 100644 index 000000000..81378a157 --- /dev/null +++ b/spec/requests/oauth/openid_connect_flow_spec.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +RSpec.describe 'OIDC authorization code flow', type: :request do + include_context :with_oauth_routes + + # :filled gives the admin a billing contact, which is where AdminUser#email + # comes from — without one the email claim is legitimately nil. + let(:admin) { create(:admin_user, :filled, roles: %w[admin noc]) } + let(:application) do + create_oauth_application( + name: 'yeti-statistics', + confidential: true, + scopes: 'openid profile email', + redirect_uri: 'https://stats.example.com/api/auth/callback' + ) + end + let(:code_verifier) { SecureRandom.urlsafe_base64(64) } + let(:code_challenge) { Base64.urlsafe_encode64(Digest::SHA256.digest(code_verifier), padding: false) } + let(:nonce) { SecureRandom.urlsafe_base64(16) } + + # As in the plain OAuth flow spec, the consent POST can't be driven from a + # request spec (CSRF), so the grant is issued directly. The nonce row is what + # Doorkeeper::OpenidConnect::OAuth::Authorization::Code would have written. + def create_authorization_code(scopes: 'openid profile email', with_nonce: true) + grant = OauthAccessGrant.create!( + application: application, + resource_owner_id: admin.id, + expires_in: 600, + redirect_uri: application.redirect_uri, + scopes: scopes, + code_challenge: code_challenge, + code_challenge_method: 'S256' + ) + OauthOpenidRequest.create!(access_grant: grant, nonce: nonce) if with_nonce + grant.plaintext_token + end + + def exchange(code) + post '/oauth/token', params: { + grant_type: 'authorization_code', + code: code, + client_id: application.uid, + client_secret: application.plaintext_secret, + redirect_uri: application.redirect_uri, + code_verifier: code_verifier + } + JSON.parse(response.body) + end + + # Verifies the signature the way a client does: against the published JWKS, + # not against the private key we happen to have on disk. + def decode_id_token(id_token) + get '/oauth/discovery/keys' + jwk = JWT::JWK.import(JSON.parse(response.body)['keys'].first) + JWT.decode(id_token, jwk.verify_key, true, algorithm: 'RS256').first + end + + context 'when the openid scope was granted' do + # let!, not let: the assertions below read `response`, which only exists + # once the exchange has actually been POSTed. + let!(:token_response) { exchange(create_authorization_code) } + let(:claims) { decode_id_token(token_response['id_token']) } + + it 'returns an id_token alongside the access token' do + expect(response).to have_http_status(:success) + expect(token_response['access_token']).to be_present + expect(token_response['id_token']).to be_present + end + + it 'signs the id_token with the published key and binds it to this client' do + expect(claims['iss']).to eq(YetiConfig.oauth.oidc.issuer) + expect(claims['aud']).to eq(application.uid) + # Replay protection: the client checks this against what it sent. + expect(claims['nonce']).to eq(nonce) + expect(claims['exp']).to be > claims['iat'] - 1 + end + + it 'identifies the admin by a stable subject' do + expect(claims['sub']).to eq(admin.id.to_s) + end + + # These are in the id_token only because every claim declares + # response: [:id_token, :user_info]. The gem's default is user_info alone, + # which would leave a client that never calls /oauth/userinfo — the normal + # case — with a token carrying nothing but sub. + it 'carries the profile claims in the id_token itself' do + expect(claims['name']).to eq(admin.username) + expect(claims['preferred_username']).to eq(admin.username) + expect(claims['email']).to eq(admin.email) + expect(claims['email_verified']).to be(true) + end + + # The authorisation boundary: clients filter on this. + it 'carries the admin roles as the groups claim' do + expect(claims['groups']).to match_array(%w[admin noc]) + end + end + + context 'without the openid scope' do + it 'issues a plain OAuth token and no id_token' do + body = exchange(create_authorization_code(scopes: 'mcp', with_nonce: false)) + expect(response).to have_http_status(:success) + expect(body['access_token']).to be_present + expect(body).not_to have_key('id_token') + end + end + + context 'when no nonce was sent' do + it 'still issues an id_token, without a nonce claim' do + body = exchange(create_authorization_code(with_nonce: false)) + expect(response).to have_http_status(:success) + expect(decode_id_token(body['id_token'])).not_to have_key('nonce') + end + end + + context 'when the admin is disabled' do + it 'refuses the exchange, so no id_token is minted' do + code = create_authorization_code + admin.update!(enabled: false) + body = exchange(code) + expect(response).to have_http_status(:bad_request) + expect(body['error']).to eq('invalid_grant') + end + end +end diff --git a/spec/requests/oauth/userinfo_spec.rb b/spec/requests/oauth/userinfo_spec.rb new file mode 100644 index 000000000..86d22eab8 --- /dev/null +++ b/spec/requests/oauth/userinfo_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +RSpec.describe 'OIDC userinfo', type: :request do + include_context :with_oauth_routes + + let(:admin) { create(:admin_user, :filled, roles: %w[admin]) } + let(:application) { create_oauth_application(confidential: true, scopes: 'openid profile email') } + let(:token) { issue_access_token(admin: admin, application: application, scopes: 'openid profile email') } + + subject do + get '/oauth/userinfo', headers: { 'Authorization' => "Bearer #{token.plaintext_token}" } + end + + it 'returns the same claims as the id_token' do + subject + expect(response).to have_http_status(:success) + payload = JSON.parse(response.body) + expect(payload['sub']).to eq(admin.id.to_s) + expect(payload['name']).to eq(admin.username) + expect(payload['preferred_username']).to eq(admin.username) + expect(payload['email']).to eq(admin.email) + expect(payload['groups']).to match_array(%w[admin]) + end + + it 'rejects a token whose admin has been disabled' do + token + admin.update!(enabled: false) + subject + # resource_owner_from_access_token filters on enabled, so the owner no + # longer resolves and the gem cannot build a response. + expect(response).not_to have_http_status(:success) + end + + it 'rejects an unauthenticated request' do + get '/oauth/userinfo' + expect(response).to have_http_status(:unauthorized) + end +end diff --git a/spec/requests/oauth/well_known_spec.rb b/spec/requests/oauth/well_known_spec.rb index b6fd0b894..04d72ff13 100644 --- a/spec/requests/oauth/well_known_spec.rb +++ b/spec/requests/oauth/well_known_spec.rb @@ -19,4 +19,26 @@ expect(payload['grant_types_supported']).to include('authorization_code', 'refresh_token') expect(payload['response_types_supported']).to include('code') end + + # The OIDC gem's discovery routes claim this path too, and its version of the + # document has no registration_endpoint — which is how MCP clients + # self-register. config/routes.rb keeps our route first; this pins it, since + # nothing else would fail if the ordering were swapped. + it 'is served by yeti\'s own controller, not the OIDC gem\'s' do + expect(Rails.application.routes.recognize_path('/.well-known/oauth-authorization-server')) + .to include(controller: 'well_known/oauth_authorization_server') + + subject + payload = JSON.parse(response.body) + expect(payload['registration_endpoint']).to end_with('/oauth/register') + expect(payload['service_documentation']).to be_present + end + + it 'advertises the OIDC endpoints when yeti is also an OIDC provider' do + subject + payload = JSON.parse(response.body) + expect(payload['jwks_uri']).to end_with('/oauth/discovery/keys') + expect(payload['userinfo_endpoint']).to end_with('/oauth/userinfo') + expect(payload['id_token_signing_alg_values_supported']).to include('RS256') + end end From 6142b1dd9e3116b20c7f36005da258830dcbc656 Mon Sep 17 00:00:00 2001 From: sdi Date: Sat, 1 Aug 2026 16:45:50 +0300 Subject: [PATCH 2/5] fixes --- Gemfile.lock | 106 +++++++++++++++++------------------ spec/config/yeti_web_spec.rb | 7 ++- 2 files changed, 59 insertions(+), 54 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 6329f1258..1f18e05da 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -105,29 +105,29 @@ GEM specs: action_text-trix (2.1.19) railties - actioncable (8.1.3) - actionpack (= 8.1.3) - activesupport (= 8.1.3) + actioncable (8.1.3.1) + actionpack (= 8.1.3.1) + activesupport (= 8.1.3.1) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (8.1.3) - actionpack (= 8.1.3) - activejob (= 8.1.3) - activerecord (= 8.1.3) - activestorage (= 8.1.3) - activesupport (= 8.1.3) + actionmailbox (8.1.3.1) + actionpack (= 8.1.3.1) + activejob (= 8.1.3.1) + activerecord (= 8.1.3.1) + activestorage (= 8.1.3.1) + activesupport (= 8.1.3.1) mail (>= 2.8.0) - actionmailer (8.1.3) - actionpack (= 8.1.3) - actionview (= 8.1.3) - activejob (= 8.1.3) - activesupport (= 8.1.3) + actionmailer (8.1.3.1) + actionpack (= 8.1.3.1) + actionview (= 8.1.3.1) + activejob (= 8.1.3.1) + activesupport (= 8.1.3.1) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (8.1.3) - actionview (= 8.1.3) - activesupport (= 8.1.3) + actionpack (8.1.3.1) + actionview (= 8.1.3.1) + activesupport (= 8.1.3.1) nokogiri (>= 1.8.5) rack (>= 2.2.4) rack-session (>= 1.0.1) @@ -135,16 +135,16 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (8.1.3) + actiontext (8.1.3.1) action_text-trix (~> 2.1.15) - actionpack (= 8.1.3) - activerecord (= 8.1.3) - activestorage (= 8.1.3) - activesupport (= 8.1.3) + actionpack (= 8.1.3.1) + activerecord (= 8.1.3.1) + activestorage (= 8.1.3.1) + activesupport (= 8.1.3.1) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (8.1.3) - activesupport (= 8.1.3) + actionview (8.1.3.1) + activesupport (= 8.1.3.1) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) @@ -171,28 +171,28 @@ GEM kaminari (>= 1.2.1) railties (>= 6.1) ransack (>= 4.0) - activejob (8.1.3) - activesupport (= 8.1.3) + activejob (8.1.3.1) + activesupport (= 8.1.3.1) globalid (>= 0.3.6) - activemodel (8.1.3) - activesupport (= 8.1.3) + activemodel (8.1.3.1) + activesupport (= 8.1.3.1) activemodel-serializers-xml (1.0.3) activemodel (>= 5.0.0.a) activesupport (>= 5.0.0.a) builder (~> 3.1) - activerecord (8.1.3) - activemodel (= 8.1.3) - activesupport (= 8.1.3) + activerecord (8.1.3.1) + activemodel (= 8.1.3.1) + activesupport (= 8.1.3.1) timeout (>= 0.4.0) activerecord-import (2.2.0) activerecord (>= 4.2) - activestorage (8.1.3) - actionpack (= 8.1.3) - activejob (= 8.1.3) - activerecord (= 8.1.3) - activesupport (= 8.1.3) + activestorage (8.1.3.1) + actionpack (= 8.1.3.1) + activejob (= 8.1.3.1) + activerecord (= 8.1.3.1) + activesupport (= 8.1.3.1) marcel (~> 1.0) - activesupport (8.1.3) + activesupport (8.1.3.1) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) @@ -836,20 +836,20 @@ GEM rack (>= 1.3) rackup (2.3.1) rack (>= 3) - rails (8.1.3) - actioncable (= 8.1.3) - actionmailbox (= 8.1.3) - actionmailer (= 8.1.3) - actionpack (= 8.1.3) - actiontext (= 8.1.3) - actionview (= 8.1.3) - activejob (= 8.1.3) - activemodel (= 8.1.3) - activerecord (= 8.1.3) - activestorage (= 8.1.3) - activesupport (= 8.1.3) + rails (8.1.3.1) + actioncable (= 8.1.3.1) + actionmailbox (= 8.1.3.1) + actionmailer (= 8.1.3.1) + actionpack (= 8.1.3.1) + actiontext (= 8.1.3.1) + actionview (= 8.1.3.1) + activejob (= 8.1.3.1) + activemodel (= 8.1.3.1) + activerecord (= 8.1.3.1) + activestorage (= 8.1.3.1) + activesupport (= 8.1.3.1) bundler (>= 1.15.0) - railties (= 8.1.3) + railties (= 8.1.3.1) rails-dom-testing (2.3.0) activesupport (>= 5.0.0) minitest @@ -861,9 +861,9 @@ GEM rack railties (>= 5.1) semantic_logger (~> 4.16) - railties (8.1.3) - actionpack (= 8.1.3) - activesupport (= 8.1.3) + railties (8.1.3.1) + actionpack (= 8.1.3.1) + activesupport (= 8.1.3.1) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) diff --git a/spec/config/yeti_web_spec.rb b/spec/config/yeti_web_spec.rb index e7504c26d..7888f38bf 100644 --- a/spec/config/yeti_web_spec.rb +++ b/spec/config/yeti_web_spec.rb @@ -99,7 +99,12 @@ tmpdir: a_kind_of(String), admin_ui: be_kind_of(Hash), oauth: { - enabled: boolean + enabled: boolean, + oidc: { + enabled: boolean, + issuer: a_kind_of(String), + signing_key_path: a_kind_of(String) + } }, mcp: { enabled: boolean From 0e5546ad1436743efb1301bcf302a053c1f44fcf Mon Sep 17 00:00:00 2001 From: sdi Date: Tue, 4 Aug 2026 17:35:34 +0300 Subject: [PATCH 3/5] fixes --- .gitignore | 3 +- Makefile | 32 ++- OIDC.md | 225 ++++++++++++++++++ app/admin/system/oauth_applications.rb | 20 +- .../oauth/registrations_controller.rb | 33 ++- .../oauth_authorization_server_controller.rb | 23 +- config/initializers/config.rb | 18 +- .../initializers/doorkeeper_openid_connect.rb | 4 +- config/locales/doorkeeper.en.yml | 14 ++ config/yeti_web.yml.ci | 14 +- config/yeti_web.yml.development | 2 +- config/yeti_web.yml.distr | 11 +- lib/tasks/oauth.rake | 19 +- spec/config/yeti_web_spec.rb | 2 +- spec/fixtures/oidc_test_signing_key.pem | 28 --- .../oauth/authorization_code_flow_spec.rb | 15 ++ .../oauth/dynamic_registration_spec.rb | 38 +++ .../oauth/openid_configuration_spec.rb | 2 +- .../oauth/openid_connect_flow_spec.rb | 2 +- spec/requests/oauth/userinfo_spec.rb | 25 ++ spec/requests/oauth/well_known_spec.rb | 33 ++- 21 files changed, 498 insertions(+), 65 deletions(-) create mode 100644 OIDC.md delete mode 100644 spec/fixtures/oidc_test_signing_key.pem diff --git a/.gitignore b/.gitignore index fe1be71f8..195d9f9fe 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,8 @@ config/secrets.yml config/ldap.yml config/oidc.yml # Signs the id_tokens yeti issues as an OIDC provider — reading it is enough to -# forge an identity for any admin. Deploy it out of band. +# forge an identity for any admin. Deploy it out of band. The test suite uses +# this same path, generated by the Makefile's $(oidc_test_key) rule. config/oidc_signing_key.pem coverage debian/files diff --git a/Makefile b/Makefile index 1897e5952..6d1e76eef 100644 --- a/Makefile +++ b/Makefile @@ -26,6 +26,7 @@ exclude_files := config/database.yml \ config/click_house.yml \ config/policy_roles.yml \ config/secrets.yml \ + config/oidc_signing_key.pem \ config/cdr_processors.yml.distr \ *.o \ *.a @@ -42,6 +43,12 @@ bundle_bin := $(gems)/bin/bundle bundler_gems := $(CURDIR)/vendor/bundle export GEM_PATH := $(gems):$(bundler_gems) +# RSA key for the OIDC provider, generated rather than committed. Same path an +# operator deploys to, so a dev box has one key for `rails s` and for the specs. +# It lives in config/, which $(app_files) packages — $(exclude_files) is what +# keeps it out of the .deb, and must stay that way. +oidc_test_key := config/oidc_signing_key.pem + # must match final destination in debian package export RBENV_ROOT := $(app_dir)/vendor/rbenv export PATH := $(RBENV_ROOT)/shims:$(PATH) @@ -112,6 +119,16 @@ config/policy_roles.yml: $(info:msg=Creating policy_roles.yml for build/tests) cp config/policy_roles.yml.distr config/policy_roles.yml +$(oidc_test_key): + $(info:msg=Generating OIDC signing key for build/tests) + @# yeti_web.yml.ci turns the OIDC provider on, and the initializer refuses to + @# boot without a readable signing key — so every target that loads the Rails + @# environment needs this first. Generated by the same rake task operators + @# run in production, so the key here has the parameters a real one does. + @# It signs nothing but test tokens; a private key committed to the repo + @# trips secret scanning regardless of what it protects. + $(bundle_bin) exec rake 'oauth:oidc:generate_signing_key[$(oidc_test_key)]' + $(RBENV_ROOT)/versions/$(rbenv_version): $(info:msg=Installing ruby $(rbenv_version) into $(RBENV_ROOT)) @@ -155,7 +172,7 @@ gems-test: bundler .PHONY: docs -docs: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml config/secrets.yml config/click_house.yml +docs: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml $(oidc_test_key) config/secrets.yml config/click_house.yml $(info:msg=Preparing test database for docs generation) RAILS_ENV=test $(bundle_bin) exec rake \ db:drop \ @@ -170,14 +187,14 @@ docs: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml .PHONY: assets -assets: gems config/database.yml config/yeti_web.yml config/policy_roles.yml config/secrets.yml +assets: gems config/database.yml config/yeti_web.yml config/policy_roles.yml $(oidc_test_key) config/secrets.yml $(info:msg=Precompile assets) RAILS_ENV=production RAILS_COMPILE_ASSETS=true $(bundle_bin) exec rake assets:precompile .PHONY: prepare-test-db -prepare-test-db: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml +prepare-test-db: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml $(oidc_test_key) $(info:msg=Preparing test database) RAILS_ENV=test $(bundle_bin) exec rake parallel:drop @# avoid race condition when createing pgq roles in parallel with @@ -204,7 +221,7 @@ test: lint audit brakeman rspec .PHONY: rspec -rspec: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml prepare-test-db config/click_house.yml config/secrets.yml +rspec: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml $(oidc_test_key) prepare-test-db config/click_house.yml config/secrets.yml ifdef spec $(info:msg=Testing spec $(spec)) RAILS_ENV=test $(bundle_bin) exec rspec "$(spec)" @@ -229,12 +246,12 @@ else endif .PHONY: database_consistency -database_consistency: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml config/secrets.yml prepare-test-db +database_consistency: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml $(oidc_test_key) config/secrets.yml prepare-test-db $(info:msg=Check the consistency of the database constraints with the application validations) RAILS_ENV=test $(bundle_bin) exec database_consistency .PHONY: annotations -annotations: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml config/secrets.yml prepare-test-db +annotations: gems-test config/database.yml config/yeti_web.yml config/policy_roles.yml $(oidc_test_key) config/secrets.yml prepare-test-db $(info:msg=Check that model annotations are up to date) RAILS_ENV=test $(bundle_bin) exec annotaterb models --frozen @@ -280,7 +297,8 @@ clean: rm -fv config/database.yml \ config/yeti_web.yml \ config/policy_roles.yml \ - config/secrets.yml + config/secrets.yml \ + $(oidc_test_key) rm -fv bin/rspec diff --git a/OIDC.md b/OIDC.md new file mode 100644 index 000000000..bcc8948ea --- /dev/null +++ b/OIDC.md @@ -0,0 +1,225 @@ +# yeti-web as an OpenID Connect provider + +yeti-web's Doorkeeper is an OAuth 2.0 provider: it answers *"may this client call +the API?"*. With `oauth.oidc.enabled` it also answers *"who is the user?"* — it +issues a signed `id_token`, publishes a JWKS and a discovery document, and serves +`userinfo`. `admin_users` then become the login for other components: +yeti-statistics, Grafana, anything that speaks OIDC. + +Everything below is off unless you turn it on. An existing OAuth-only deployment +behaves exactly as before. + +--- + +## When *not* to use this + +If yeti-web already signs its own admins in through an external IdP (Keycloak, +Authentik, Okta…) — i.e. `config/oidc.yml` exists — point the other component at +**that same issuer** instead. Two lines of config, no key to manage: + +```yaml +# yeti-statistics config.yml +auth: + enabled: true + issuer: https:// + client_id: yeti-statistics +``` + +Both apps become clients of one IdP, which is what single sign-on *is*. Making +yeti-web an IdP that itself federates to another IdP inserts a pointless hop and +leaves you maintaining token signing and key rotation that a purpose-built IdP +already does better. + +Use this document when `AdminUser` with a local password is the real login. + +--- + +## What you get + +| Endpoint | Serves | +|---|---| +| `/.well-known/openid-configuration` | OIDC discovery — clients read everything else from here | +| `/oauth/discovery/keys` | JWKS: the public half of the signing key, with a `kid` | +| `/oauth/userinfo` | Claims for a bearer token | +| `/oauth/authorize`, `/oauth/token` | Unchanged; the token response gains an `id_token` when `openid` was granted | +| `/.well-known/oauth-authorization-server` | Unchanged RFC 8414 document for MCP clients, now also naming the OIDC endpoints | + +Claims in both the `id_token` and `userinfo`: + +| Claim | Source | +|---|---| +| `sub` | `AdminUser#id` — stable, never recycled | +| `name`, `preferred_username` | `AdminUser#username` | +| `email` | `AdminUser#email`, which comes off the billing contact — **nil if the admin has none** | +| `email_verified` | whether that address exists | +| `groups` | `AdminUser#roles` — this is what clients authorise on | + +--- + +## Setup + +### 1. Generate the signing key + +```sh +bundle exec rake 'oauth:oidc:generate_signing_key[/etc/yeti-web/oidc_signing_key.pem]' +``` + +2048-bit RSA, written at mode 0400. **This key mints identities**: anyone who can +read it can forge an `id_token` for any admin on every client that trusts this +issuer. Deploy it out of band (Ansible, not git) and treat it like the database +password. `config/oidc_signing_key.pem` is gitignored for the same reason. + +### 2. Enable it + +```yaml +# config/yeti_web.yml +oauth: + enabled: true + issuer: https://web.example.com + oidc: + enabled: true + signing_key_path: /etc/yeti-web/oidc_signing_key.pem +``` + +`issuer` sits on `oauth`, not on `oauth.oidc`, because it identifies the +authorization server itself: the same string is reported by +`/.well-known/oauth-authorization-server` and `/.well-known/openid-configuration` +and carried in every `id_token`. One server, one identity — a client that +discovers through either document must see the same answer. + +It must be the URL clients actually reach yeti-web on. Clients compare it against +the discovery document, against the URL they fetched it from, and against the +`iss` claim — a trailing slash is enough to break every login. It is mandatory +once `oidc.enabled`, and yeti-web refuses to boot without it (or if the key is +unreadable). With OAuth alone it may be omitted, in which case the RFC 8414 +document reports the URL the request came in on — which is what lets MCP +one-click connect work with no configuration at all. + +Run the migration if this is an upgrade: the `nonce` a client sends has to +survive between the authorization request and the token exchange, and it lives in +`gui.oauth_openid_requests`. + +### 3. Register the client + +In the admin UI: **System → Admin Access → OAuth Applications → New**. Give it a +name, a redirect URI and the `openid profile email` scopes; leave Client ID and +Client secret blank to have them generated, or set them to fixed values when the +client's config is deployed from a template. The detail page shows both +afterwards, and has a **Rotate secret** action. + +The page is root-only until a role is granted the `System/OauthApplication` +section in `config/policy_roles.yml` — it displays client secrets in cleartext, +which is how Doorkeeper stores them. + +The `redirect_uri` must match what the client sends **byte for byte**, including +any base path (`https://stats.example.com/stats/api/auth/callback`), and must be +HTTPS unless the host is a loopback address. + +MCP clients (Claude Code, Cursor) need none of this — they self-register through +`POST /oauth/register`, and appear in the same list once they have. OIDC clients +cannot: that endpoint is unauthenticated, so it grants the `mcp` scope only and +rejects a registration asking for `openid`, `profile` or `email` with +`invalid_client_metadata`. Anything that signs users in is registered here, by an +operator. + +### 4. Configure the client + +```yaml +# yeti-statistics config.yml +auth: + enabled: true + issuer: https://web.example.com + client_id: + client_secret: + redirect_url: https://stats.example.com/api/auth/callback + cookie_secret: + allowed_groups: [admin] # matched against AdminUser#roles +``` + +**Set `allowed_groups`.** Without it, any account this provider knows — every +enabled admin — can read every customer's traffic and margin. + +--- + +## Verification + +```sh +# 1. Discovery, and the issuer it claims. +curl -s https://web.example.com/.well-known/openid-configuration | jq +# issuer must equal what the client is configured with, exactly. +# Also check: jwks_uri, id_token_signing_alg_values_supported ["RS256"], +# code_challenge_methods_supported ["S256"], scopes_supported incl. "openid". + +# 2. JWKS serves a public key with a kid. +curl -s https://web.example.com/oauth/discovery/keys | jq + +# 3. MCP discovery is untouched — registration_endpoint must still be there. +curl -s https://web.example.com/.well-known/oauth-authorization-server | jq +``` + +Then run the real flow: open yeti-statistics, click **Sign in**. You should land +on the ActiveAdmin login page and come back signed in. + +When it fails, the client's error says which step broke: + +| error | cause | +|---|---| +| `provider returned no id_token: it is OAuth2, not OIDC` | `oauth.oidc.enabled` is off, or the client's registered scopes don't include `openid` | +| `id_token: issuer did not match` | `oauth.issuer` ≠ the client's `issuer` | +| `id_token: failed to verify signature` | JWKS unreachable, or the key was rotated with no overlap | +| `nonce mismatch` | the `gui.oauth_openid_requests` migration has not been run | +| `account "x" is in none of the permitted groups` | `AdminUser#roles` contains none of the client's `allowed_groups` | +| yeti-web won't boot, complaining about `oauth.issuer` / `oauth.oidc.signing_key_path` | step 1 or 2 is incomplete — the message says which | + +--- + +## Operating it + +**Key rotation.** Publish the new key in JWKS *before* signing with it, keep the +old one published until every issued token has expired, then drop the old one. +The `kid` header on each token says which key signed it. Skipping the overlap +signs everyone out. + +**Offboarding.** Setting `enabled = false` on an `AdminUser` takes effect +immediately: `OauthAccessToken#accessible?` checks the owner, so the token stops +working at `userinfo`, at introspection and at `/api/mcp` on the next request, +and `before_successful_strategy_response` blocks any new token or refresh. An +`id_token` already handed to a client stays valid until that client's own session +expires (yeti-statistics: `session_ttl`, 12h by default) — shorten it there if +immediate revocation matters. + +**Scopes.** `openid` is optional and never granted by default; a client has to +ask for it. Withholding it is also what keeps the whole OIDC layer dormant when +the feature is off. + +--- + +## Implementation notes + +Three things here are load-bearing and not obvious from the gem's README: + +- **The gem is configured unconditionally** + (`config/initializers/doorkeeper_openid_connect.rb`), even when the feature is + off. It prepends an `openid_request` association onto every Doorkeeper access + grant model, and that association reads + `Doorkeeper::OpenidConnect.configuration` while the class body is evaluated — + so leaving it unconfigured makes `OauthAccessGrant` raise at eager load, in + every deployment. What the flag actually gates is the `openid` scope and the + routes. +- **Claims declare `response: [:id_token, :user_info]`.** The gem's default is + `user_info` alone, which would leave a client that never calls + `/oauth/userinfo` — the normal case — with a token carrying nothing but `sub`. +- **Route order in `config/routes.rb` matters.** The OIDC gem's discovery routes + claim `/.well-known/oauth-authorization-server` too, and its version of that + document has no `registration_endpoint`. yeti's own route is declared first so + MCP clients can still self-register. + +`use_doorkeeper` also skips Doorkeeper's built-in `:applications` controller. +That UI was already unreachable (`admin_authenticator` answers 403), and leaving +it mounted claimed the `oauth_application(s)` route-helper names that the +ActiveAdmin page needs for its own links. + +**RP-initiated logout** (`end_session_endpoint`) is not implemented. Clients +default to dropping their own session and leaving yeti-web's alone, which is the +right default: a "sign out" button in a stats dashboard should not sign the user +out of everything on the issuer. diff --git a/app/admin/system/oauth_applications.rb b/app/admin/system/oauth_applications.rb index 76f1e0b94..7fe346ff7 100644 --- a/app/admin/system/oauth_applications.rb +++ b/app/admin/system/oauth_applications.rb @@ -6,13 +6,23 @@ # # MCP clients (Claude Code, Cursor, ...) do not need this page: they self-register # through POST /oauth/register (RFC 7591) as public PKCE clients. They still show -# up in the list once they have. +# up in the list once they have. That endpoint is unauthenticated, so it hands out +# the MCP scope and nothing else — the OIDC scopes carry the admin's email and +# roles and can only be granted here. See SELF_REGISTRABLE_SCOPES in +# app/controllers/oauth/registrations_controller.rb. # -# Access is role-gated by OauthApplicationPolicy, and root-only until some role -# is granted the "System/OauthApplication" section in config/policy_roles.yml. -# That default matters more here than on most pages: the show page displays the +# Access is role-gated by OauthApplicationPolicy through the +# "System/OauthApplication" section of config/policy_roles.yml. A role with no +# such section falls back to its "Default" section, and the shipped template +# (config/policy_roles.yml.distr) gives `user` a permissive Default — so `user` +# manages clients out of the box, which is intended: it is an administrator role +# here. `reporter` reaches the page read-only by the same fallback. +# +# Read access matters more here than on most pages: the show page displays the # client secret in cleartext — which is how Doorkeeper stores it, and which an -# operator configuring a client has to be able to read back. +# operator configuring a client has to be able to read back. A deployment that +# wants either role kept off the page adds an explicit +# "System/OauthApplication" section for it. ActiveAdmin.register OauthApplication do menu parent: ['System', 'Admin Access'], label: 'OAuth Applications', priority: 98 diff --git a/app/controllers/oauth/registrations_controller.rb b/app/controllers/oauth/registrations_controller.rb index 204ce8c8a..9f4853327 100644 --- a/app/controllers/oauth/registrations_controller.rb +++ b/app/controllers/oauth/registrations_controller.rb @@ -10,6 +10,22 @@ class RegistrationsController < ActionController::API # are supported. `nil` / missing = public client (treated as 'none'). SUPPORTED_AUTH_METHODS = %w[none client_secret_basic].freeze + # What a stranger may register itself for. This endpoint exists to serve MCP + # clients, so it grants the MCP scope and nothing else — in particular not + # the OIDC scopes (doorkeeper.rb), which carry the admin's email and roles + # and are meant to be registered by an operator through the admin UI (see + # app/admin/system/oauth_applications.rb). Without this the split is only a + # convention: anyone could self-register a plausibly-named client asking for + # `openid profile email` and collect an identity from the first admin who + # approves what looks like an ordinary sign-in prompt. + # + # Enforced here rather than at /oauth/authorize because this is the only + # unauthenticated way to create an application: the admin UI is authenticated + # and root-gated, Doorkeeper's own applications controller is not mounted + # (skip_controllers :applications), and there is no RFC 7592 endpoint to + # widen a client's scopes afterwards. + SELF_REGISTRABLE_SCOPES = %w[mcp].freeze + def create params = JSON.parse(request.body.read) @@ -21,10 +37,25 @@ def create }, status: 400 end + # RFC 7591 §3.2.1 also allows quietly returning a narrower `scope` than was + # asked for, but a client that is told what it may have can act on it, + # whereas one handed a silently trimmed scope only finds out later, at the + # token endpoint, with nothing to point at. + requested_scopes = params['scope'].to_s.split + unsupported_scopes = requested_scopes - SELF_REGISTRABLE_SCOPES + if unsupported_scopes.any? + return render json: { + error: 'invalid_client_metadata', + error_description: "Unsupported scope: #{unsupported_scopes.join(' ')}. " \ + "Dynamic registration may request: #{SELF_REGISTRABLE_SCOPES.join(' ')}. " \ + 'Other scopes are registered by an administrator.' + }, status: 400 + end + app = OauthApplication.new( name: params['client_name'].to_s[0, 100].presence || 'Unnamed client', redirect_uri: Array(params['redirect_uris']).join("\n"), - scopes: params['scope'].presence || Doorkeeper.config.default_scopes.to_s, + scopes: requested_scopes.presence&.join(' ') || Doorkeeper.config.default_scopes.to_s, confidential: auth_method != 'none' ) diff --git a/app/controllers/well_known/oauth_authorization_server_controller.rb b/app/controllers/well_known/oauth_authorization_server_controller.rb index 877b3f43d..488c5f3bc 100644 --- a/app/controllers/well_known/oauth_authorization_server_controller.rb +++ b/app/controllers/well_known/oauth_authorization_server_controller.rb @@ -7,7 +7,7 @@ module WellKnown class OauthAuthorizationServerController < ActionController::API def show render json: { - issuer: request.base_url, + issuer: issuer, authorization_endpoint: "#{request.base_url}/oauth/authorize", token_endpoint: "#{request.base_url}/oauth/token", registration_endpoint: "#{request.base_url}/oauth/register", @@ -24,6 +24,27 @@ def show private + # The one identity this authorization server claims — the same string the + # OIDC discovery document reports and every id_token carries in `iss`, so a + # client that discovers here and validates a token there sees no mismatch. + # Emitted verbatim: clients compare it byte for byte, so normalising a + # trailing slash away here would break exactly the logins it looks like it + # fixes. + # + # Falls back to the request's own base URL when unset, which is the MCP-only + # deployment: oauth.enabled with no oidc block, nothing to compare against, + # zero config. oauth.issuer is mandatory once OIDC is on — see + # config/initializers/doorkeeper_openid_connect.rb. + # + # The endpoints below deliberately stay on request.base_url: the OIDC gem + # builds its own discovery document from Rails URL helpers, i.e. from the + # request, and two documents advertising different hosts for /oauth/authorize + # would be a worse failure than the one this avoids. If a proxy really does + # rewrite host or scheme, fix it with X-Forwarded-* / default_url_options. + def issuer + YetiConfig.oauth&.issuer.presence || request.base_url + end + # When yeti is also an OIDC provider, say so here. A client that reads only # this document (it is served at a path the OIDC gem would otherwise claim # — see config/routes.rb) should not conclude there is no OIDC on offer. diff --git a/config/initializers/config.rb b/config/initializers/config.rb index 31d35cebd..72e567cfa 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -97,14 +97,24 @@ def self.setting_files(config_root, _env) optional(:oauth).schema do optional(:enabled).value(:bool?) + # How yeti identifies itself as an authorization server, in both + # /.well-known/oauth-authorization-server and, when OIDC is on, + # /.well-known/openid-configuration and every id_token. One server, one + # identity — the two documents must never disagree. + # + # Optional: the RFC 8414 document falls back to the request's own base + # URL, which is what keeps MCP one-click connect zero-config. Mandatory + # once oauth.oidc.enabled, since an id_token's `iss` is compared byte for + # byte by the client and cannot be derived per request. That is enforced + # in the initializer rather than here so the message can say what to do + # about it. + optional(:issuer).maybe(:string) + # Turns the OAuth provider into an OIDC provider as well: id_token, # /.well-known/openid-configuration, JWKS and userinfo. Requires - # oauth.enabled. `issuer` and `signing_key_path` are mandatory once - # enabled, but that is enforced in the initializer rather than here so - # the message can say what to do about it. + # oauth.enabled. optional(:oidc).schema do optional(:enabled).value(:bool?) - optional(:issuer).maybe(:string) optional(:signing_key_path).maybe(:string) end end diff --git a/config/initializers/doorkeeper_openid_connect.rb b/config/initializers/doorkeeper_openid_connect.rb index cd2773a0e..9db6d4919 100644 --- a/config/initializers/doorkeeper_openid_connect.rb +++ b/config/initializers/doorkeeper_openid_connect.rb @@ -28,9 +28,9 @@ oidc_signing_key = nil if oidc_enabled - oidc_issuer = YetiConfig.oauth.oidc.issuer.presence + oidc_issuer = YetiConfig.oauth.issuer.presence if oidc_issuer.nil? - raise 'yeti_web.yml: oauth.oidc.issuer is required when oauth.oidc.enabled. ' \ + raise 'yeti_web.yml: oauth.issuer is required when oauth.oidc.enabled. ' \ 'It must equal, byte for byte, the issuer configured on every client — ' \ 'clients compare it against the discovery document and the id_token, ' \ 'and a trailing slash is enough to break every login.' diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 139cd3857..b241c25c8 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -19,6 +19,20 @@ en: not_match_configured: "doesn't match those configured on the server." doorkeeper: + # One line per scope on the consent screen, under "This application will be + # able to:". Whatever is missing here renders as a translation_missing span, + # so every scope the server offers needs an entry. + scopes: + mcp: 'Use the MCP API as you: read CDRs and simulate routing' + # Overrides the OIDC gem's "Authenticate your account", which is true but + # incomplete: the `groups` claim rides on this scope (see + # config/initializers/doorkeeper_openid_connect.rb) and carries the admin + # roles a client authorises against. An admin approving a sign-in should + # see that they are handing over their roles, not just their identity. + openid: 'Sign you in, and share your username and admin roles' + # profile / email keep the gem's wording — "View your profile information" + # and "View your email address" — which describe those claims accurately. + applications: confirmations: destroy: 'Are you sure?' diff --git a/config/yeti_web.yml.ci b/config/yeti_web.yml.ci index b44e14f10..2285bc7b2 100644 --- a/config/yeti_web.yml.ci +++ b/config/yeti_web.yml.ci @@ -43,12 +43,18 @@ sentry: oauth: enabled: true # mount Doorkeeper OAuth provider — required for the spec suite - # OIDC provider — required for the spec suite. The key below is a throwaway - # committed for the tests; it signs nothing real. + # Deliberately NOT the host request specs run against (http://www.example.com), + # so every `iss` assertion actually discriminates between the configured issuer + # and the request's own base URL. + issuer: https://web.example.com + + # OIDC provider — required for the spec suite. The key is generated by the + # Makefile ($(oidc_test_key)) before anything boots Rails, never committed: it + # is a throwaway that signs nothing real, but a private key in the repo trips + # secret scanning all the same. oidc: enabled: true - issuer: http://www.example.com - signing_key_path: spec/fixtures/oidc_test_signing_key.pem + signing_key_path: config/oidc_signing_key.pem mcp: enabled: true # mount /api/mcp; requires oauth.enabled — required for the spec suite diff --git a/config/yeti_web.yml.development b/config/yeti_web.yml.development index b34ceecea..da11c3e7c 100644 --- a/config/yeti_web.yml.development +++ b/config/yeti_web.yml.development @@ -45,9 +45,9 @@ oauth: # OpenID Connect provider on top of it — see yeti_web.yml.distr for what each # key means. `rake oauth:oidc:generate_signing_key` writes the key. + #issuer: http://127.0.0.1:3000 #oidc: # enabled: true - # issuer: http://127.0.0.1:3000 # signing_key_path: config/oidc_signing_key.pem mcp: diff --git a/config/yeti_web.yml.distr b/config/yeti_web.yml.distr index 363ca1045..ec0bdf917 100644 --- a/config/yeti_web.yml.distr +++ b/config/yeti_web.yml.distr @@ -43,6 +43,14 @@ sentry: oauth: enabled: false # mount Doorkeeper OAuth provider (/oauth/*, /.well-known/oauth-authorization-server) + # The URL clients actually reach yeti-web on. Reported as `issuer` in both + # discovery documents and carried in every id_token. Optional while only + # OAuth is enabled (the metadata document then reports the URL the request + # came in on); mandatory once oidc.enabled below, where it MUST equal, byte + # for byte, the issuer configured on the client side — a trailing slash is + # enough to break every login. + #issuer: https://web.example.com + # Also act as an OpenID Connect provider, so other yeti components # (yeti-statistics, Grafana, ...) can use admin_users as their login. Adds # /.well-known/openid-configuration, /oauth/discovery/keys and @@ -50,9 +58,6 @@ oauth: # clients that request the `openid` scope. #oidc: # enabled: true - # # MUST equal, byte for byte, the issuer configured on the client side — - # # a trailing slash is enough to break every login. - # issuer: https://web.example.com # # RS256 private key that signs the id_token. Generate with # # bundle exec rake oauth:oidc:generate_signing_key[/etc/yeti-web/oidc_signing_key.pem] # # and deploy it out of band: whoever can read it can mint an identity for diff --git a/lib/tasks/oauth.rake b/lib/tasks/oauth.rake index f88b38a0f..eb052c5bf 100644 --- a/lib/tasks/oauth.rake +++ b/lib/tasks/oauth.rake @@ -17,18 +17,29 @@ namespace :oauth do desc 'Generate the RSA key that signs id_tokens' task :generate_signing_key, [:path] do |_t, args| path = args[:path].presence || 'config/oidc_signing_key.pem' - raise ArgumentError, "refusing to overwrite existing key at #{path}" if File.exist?(path) key = OpenSSL::PKey::RSA.new(2048) - File.write(path, key.to_pem) - File.chmod(0o400, path) + + # Created at 0400 rather than written and then chmod'ed: File.write would + # apply the process umask, typically leaving the key world-readable until + # the next statement ran — a window another user on the host can read it + # in. O_EXCL makes the refusal-to-overwrite atomic too, where a preceding + # File.exist? check would be a TOCTOU. + begin + File.open(path, File::WRONLY | File::CREAT | File::EXCL, 0o400) do |f| + f.write(key.to_pem) + end + rescue Errno::EEXIST + raise ArgumentError, "refusing to overwrite existing key at #{path}" + end puts "Wrote a 2048-bit RSA private key to #{path} (mode 0400)." puts 'Point yeti_web.yml at it:' puts ' oauth:' + puts ' enabled: true' + puts ' issuer: https://web.example.com' puts ' oidc:' puts ' enabled: true' - puts ' issuer: https://web.example.com' puts " signing_key_path: #{path}" end end diff --git a/spec/config/yeti_web_spec.rb b/spec/config/yeti_web_spec.rb index 7888f38bf..a2ff771c1 100644 --- a/spec/config/yeti_web_spec.rb +++ b/spec/config/yeti_web_spec.rb @@ -100,9 +100,9 @@ admin_ui: be_kind_of(Hash), oauth: { enabled: boolean, + issuer: a_kind_of(String), oidc: { enabled: boolean, - issuer: a_kind_of(String), signing_key_path: a_kind_of(String) } }, diff --git a/spec/fixtures/oidc_test_signing_key.pem b/spec/fixtures/oidc_test_signing_key.pem deleted file mode 100644 index 524f59f61..000000000 --- a/spec/fixtures/oidc_test_signing_key.pem +++ /dev/null @@ -1,28 +0,0 @@ ------BEGIN PRIVATE KEY----- -MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCsKcSZQw2Bcqw1 -NIHqImgv2b1fGAAxOnSrGi0aTHJPQFpbisvAkE5Q2PJWIj1OvqDWVEQFVKS7cEb9 -Acx/LvdOH3Ri2E32AYze63hvFgMZbJ+yk5XtXjT3TC6qREyF5Z3fXA9hd4VVzrEq -UmyoVznNyT6xfH/0oLiA3UonvYvs848bDfrO7pSTqskde/YXO0+CctHcDMUFjlfI -T7IjIduBAzqZq/5sRNOGlkR3xzNP4iXMbQ5Fi2dRhi6OFAaDiHnrIssRV80Y6pTk -pfXcJsLiMppdOOmLTFnGIn+kkWO4q3GYkHbHSeIYtULApRbJVh/p5ul1RTXo2G4U -CVuNSV1PAgMBAAECggEAEfjsQpma29QWQRmhpEkCOliPKdGuF3WEP2ZkpfWsCzTr -GdpT6d7Gpi73oWFCqClDTgyO60WKStzuNDRPXkmPXJLxCe6NuOwxggXv1Rzlpu5N -f41jndtzSQulZXXqzSGKyQnpuyGIhEwm94WSPUKZ0K89Abc+/lEW4bD4MEzbwhkA -7sCw1jHzV1Gah/OqJNydgQHi/aGlDkqJ+2y9xL/1bvZfMEnrhpsdNhNfIgz6xB3x -ziWjr1AcibPVm+wpnWPsZc1SMbUj1WNmyslVB9NUYoF5504It8//8Y55qzdqeVRS -cKrqBh/iFvP/9NTDt7VuxnpIkCcIYc2kDPg6N5CIRQKBgQDkeUgqlbVKr4UW/dAH -aFtaVhqFRxlAx26TNqU80KRtNg9CnoyYgzo5jtG3o1Pr3K+9ZNwXydOe1y14Ogiz -dmsL8syNVJx3xHf5WSVTqA2vr1FttMkuloTaDDduip9CgnFUVvxIHPy7WcDwzJQO -SbWDB9T3YumUY+OgiHr8+BWNmwKBgQDA57jrVm/hk5Y89rWjkGgEYRv/vettWEa1 -4i655G7eO9fXxrHq/Src+kWP3RvIR2i+IJqRJ71Ad8nC2pxDq5AgNKEA/KYr6ASq -KynUPr+xTIQ/nmRRFfFcYdZwz6qiWm/uVWD3tuVv+v1EPuoUGW5qjuxujCIsWU52 -rGsFQOOEXQKBgFIu5iybSWkiFcedaPUhLqsiCcwjNQw+MKI3p9xmWJ8IMRkPmxdJ -LOqDWyCpWYigC38fBqRv7vVWBX8XsQWM9RiJL9cutlHGlDlXPbwH2VR2xd2miC9/ -6S3d8xqKNptf/JAP8hOmiaqCsIptrFwvW2FUseCC7e54t+qI7WSNgIM9AoGAQlev -LiilA7RpSAAlCeKhE1h+c57Sd+GTN/xYy4+3XXDBU7E/AS0eb68W+1o0stV0PWcj -IWbEXIaV+hCLC27We2z+LkO1toXuDMQZd6g47vX8yZBkucpNHtN7F4MkEP//XL06 -QxdAoxlzvuPQRydT5QvQXliNbCkupfu4Nq9RT/UCgYATYc7TMND7KEsUotq5l5nA -/7+qm0Mm96WOZqpjiBRCs6QhMP277D84keneqGnI1HTD0iUI0qAyZMgNpRfaA5qB -Wvj7hNHz+PCRtjnVoS9guefZIvKwtT0LpTJBCnHMFW9lN8w0HC1lqjLXX3mHPsjv -NELohGk68Fl73NjHGk3VEg== ------END PRIVATE KEY----- diff --git a/spec/requests/oauth/authorization_code_flow_spec.rb b/spec/requests/oauth/authorization_code_flow_spec.rb index 0c035aa28..59247024a 100644 --- a/spec/requests/oauth/authorization_code_flow_spec.rb +++ b/spec/requests/oauth/authorization_code_flow_spec.rb @@ -43,6 +43,21 @@ def create_authorization_code } expect(response).to have_http_status(:success) end + + # The consent screen renders one line per scope from doorkeeper.scopes.*, and + # a missing key renders a "translation missing" span rather than failing — so + # a scope added without a description reaches admins as noise, on the one + # screen where they decide what to hand over. + it 'describes every configured scope in plain English' do + Doorkeeper.config.scopes.each do |scope| + expect(I18n.exists?("doorkeeper.scopes.#{scope}")) + .to be(true), "no doorkeeper.scopes.#{scope} translation for the consent screen" + end + end + + it 'tells the admin that signing in also shares their roles' do + expect(I18n.t('doorkeeper.scopes.openid')).to match(/roles/i) + end end it 'exchanges an authorization code for an access token' do diff --git a/spec/requests/oauth/dynamic_registration_spec.rb b/spec/requests/oauth/dynamic_registration_spec.rb index 223aca3a2..8f5462a51 100644 --- a/spec/requests/oauth/dynamic_registration_spec.rb +++ b/spec/requests/oauth/dynamic_registration_spec.rb @@ -53,6 +53,44 @@ def register(body) expect(OauthApplication.find_by(name: 'Post Client')).to be_nil end + it 'grants the MCP scope when none is asked for' do + register(client_name: 'Default Scope', redirect_uris: ['https://example.test/cb']) + expect(response).to have_http_status(:created) + expect(JSON.parse(response.body)['scope']).to eq('mcp') + end + + # Self-registration exists for MCP clients. The OIDC scopes carry the admin's + # email and roles and are registered by an operator through the admin UI, so a + # stranger must not be able to mint a client that asks for them — see + # SELF_REGISTRABLE_SCOPES. + it 'rejects a self-registered client asking for the OIDC scopes' do + register( + client_name: 'Yeti Statistics', + redirect_uris: ['https://evil.test/cb'], + scope: 'openid profile email' + ) + expect(response).to have_http_status(:bad_request) + body = JSON.parse(response.body) + expect(body['error']).to eq('invalid_client_metadata') + expect(body['error_description']).to include('openid', 'profile', 'email') + expect(OauthApplication.find_by(name: 'Yeti Statistics')).to be_nil + end + + # Rejected wholesale rather than trimmed down to `mcp`: a client that asked for + # an identity and was silently given API access instead has been told nothing. + it 'rejects a mix of permitted and forbidden scopes rather than trimming it' do + register(client_name: 'Mixed', redirect_uris: ['https://example.test/cb'], scope: 'mcp openid') + expect(response).to have_http_status(:bad_request) + expect(JSON.parse(response.body)['error_description']).to include('openid') + expect(OauthApplication.find_by(name: 'Mixed')).to be_nil + end + + it 'accepts an explicit request for the MCP scope' do + register(client_name: 'Explicit', redirect_uris: ['https://example.test/cb'], scope: 'mcp') + expect(response).to have_http_status(:created) + expect(OauthApplication.find_by(name: 'Explicit').scopes.to_s).to eq('mcp') + end + it 'rejects malformed JSON with 400' do post '/oauth/register', params: 'not json', headers: { 'Content-Type' => 'application/json' } diff --git a/spec/requests/oauth/openid_configuration_spec.rb b/spec/requests/oauth/openid_configuration_spec.rb index 6c943e2e0..1791821b1 100644 --- a/spec/requests/oauth/openid_configuration_spec.rb +++ b/spec/requests/oauth/openid_configuration_spec.rb @@ -14,7 +14,7 @@ # A client compares this against the URL it was configured with and # against the id_token's iss; all three must be identical. - expect(payload['issuer']).to eq(YetiConfig.oauth.oidc.issuer) + expect(payload['issuer']).to eq(YetiConfig.oauth.issuer) expect(payload['authorization_endpoint']).to end_with('/oauth/authorize') expect(payload['token_endpoint']).to end_with('/oauth/token') diff --git a/spec/requests/oauth/openid_connect_flow_spec.rb b/spec/requests/oauth/openid_connect_flow_spec.rb index 81378a157..4a8d47296 100644 --- a/spec/requests/oauth/openid_connect_flow_spec.rb +++ b/spec/requests/oauth/openid_connect_flow_spec.rb @@ -68,7 +68,7 @@ def decode_id_token(id_token) end it 'signs the id_token with the published key and binds it to this client' do - expect(claims['iss']).to eq(YetiConfig.oauth.oidc.issuer) + expect(claims['iss']).to eq(YetiConfig.oauth.issuer) expect(claims['aud']).to eq(application.uid) # Replay protection: the client checks this against what it sent. expect(claims['nonce']).to eq(nonce) diff --git a/spec/requests/oauth/userinfo_spec.rb b/spec/requests/oauth/userinfo_spec.rb index 86d22eab8..aa4f8ddaf 100644 --- a/spec/requests/oauth/userinfo_spec.rb +++ b/spec/requests/oauth/userinfo_spec.rb @@ -22,6 +22,31 @@ expect(payload['groups']).to match_array(%w[admin]) end + # Each claim is gated on a scope, so a client gets only what it was granted. + # None of the claim definitions passes `scope:` except `groups` — the gem + # derives the rest from Claims::Claim::STANDARD_CLAIMS (`name` and + # `preferred_username` → profile, `email`/`email_verified` → email) and + # ClaimsBuilder.generate filters on it. That is a gem default carrying a + # privacy guarantee, so pin it here rather than trust it to survive an upgrade. + context 'with a token granted openid alone' do + let(:token) { issue_access_token(admin: admin, application: application, scopes: 'openid') } + + it 'returns only the claims that scope covers' do + subject + expect(response).to have_http_status(:success) + payload = JSON.parse(response.body) + + expect(payload['sub']).to eq(admin.id.to_s) + # Carried by openid on purpose: it is what a client authorises against. + expect(payload['groups']).to match_array(%w[admin]) + + expect(payload).not_to have_key('name') + expect(payload).not_to have_key('preferred_username') + expect(payload).not_to have_key('email') + expect(payload).not_to have_key('email_verified') + end + end + it 'rejects a token whose admin has been disabled' do token admin.update!(enabled: false) diff --git a/spec/requests/oauth/well_known_spec.rb b/spec/requests/oauth/well_known_spec.rb index 04d72ff13..aad87cd9e 100644 --- a/spec/requests/oauth/well_known_spec.rb +++ b/spec/requests/oauth/well_known_spec.rb @@ -9,7 +9,7 @@ subject expect(response).to have_http_status(:success) payload = JSON.parse(response.body) - expect(payload['issuer']).to be_present + expect(payload['issuer']).to eq(YetiConfig.oauth.issuer) expect(payload['authorization_endpoint']).to end_with('/oauth/authorize') expect(payload['token_endpoint']).to end_with('/oauth/token') expect(payload['registration_endpoint']).to end_with('/oauth/register') @@ -41,4 +41,35 @@ expect(payload['userinfo_endpoint']).to end_with('/oauth/userinfo') expect(payload['id_token_signing_alg_values_supported']).to include('RS256') end + + # Since this document also advertises jwks_uri and userinfo_endpoint, a client + # may discover here and then validate an id_token minted against the OIDC + # document's issuer. One authorization server, one identity — if these two ever + # disagree, every such login fails with an issuer mismatch while + # /.well-known/openid-configuration looks perfectly healthy. + it 'claims the same issuer as the OIDC discovery document' do + subject + rfc8414 = JSON.parse(response.body) + + get '/.well-known/openid-configuration' + openid = JSON.parse(response.body) + + expect(rfc8414['issuer']).to eq(openid['issuer']) + # ...and it is the configured issuer, not the host the request came in on — + # yeti_web.yml.ci sets the two to different values on purpose. + expect(rfc8414['issuer']).to eq(YetiConfig.oauth.issuer) + expect(rfc8414['issuer']).not_to eq('http://www.example.com') + end + + # The MCP-only deployment: oauth.enabled with no issuer configured. Nothing to + # compare an id_token against, so the request's own base URL is the honest + # answer — and it keeps one-click connect working with zero config. + context 'when oauth.issuer is not configured' do + before { allow(YetiConfig.oauth).to receive(:issuer).and_return(nil) } + + it 'falls back to the request base URL' do + subject + expect(JSON.parse(response.body)['issuer']).to eq('http://www.example.com') + end + end end From 6a1f782fe6801cf6903ba5c3ffc09166edd47140 Mon Sep 17 00:00:00 2001 From: sdi Date: Tue, 4 Aug 2026 19:20:16 +0300 Subject: [PATCH 4/5] remove comments --- OIDC.md | 225 ------------------ app/admin/system/oauth_applications.rb | 43 +--- app/policies/oauth_application_policy.rb | 9 - app/services/mcp/server.rb | 2 +- config/initializers/config.rb | 17 +- .../initializers/doorkeeper_openid_connect.rb | 65 ++--- config/locales/doorkeeper.en.yml | 12 +- config/yeti_web.yml.ci | 9 +- config/yeti_web.yml.distr | 13 +- .../system/oauth_applications_spec.rb | 10 +- .../oauth/openid_connect_flow_spec.rb | 2 +- 11 files changed, 33 insertions(+), 374 deletions(-) delete mode 100644 OIDC.md diff --git a/OIDC.md b/OIDC.md deleted file mode 100644 index bcc8948ea..000000000 --- a/OIDC.md +++ /dev/null @@ -1,225 +0,0 @@ -# yeti-web as an OpenID Connect provider - -yeti-web's Doorkeeper is an OAuth 2.0 provider: it answers *"may this client call -the API?"*. With `oauth.oidc.enabled` it also answers *"who is the user?"* — it -issues a signed `id_token`, publishes a JWKS and a discovery document, and serves -`userinfo`. `admin_users` then become the login for other components: -yeti-statistics, Grafana, anything that speaks OIDC. - -Everything below is off unless you turn it on. An existing OAuth-only deployment -behaves exactly as before. - ---- - -## When *not* to use this - -If yeti-web already signs its own admins in through an external IdP (Keycloak, -Authentik, Okta…) — i.e. `config/oidc.yml` exists — point the other component at -**that same issuer** instead. Two lines of config, no key to manage: - -```yaml -# yeti-statistics config.yml -auth: - enabled: true - issuer: https:// - client_id: yeti-statistics -``` - -Both apps become clients of one IdP, which is what single sign-on *is*. Making -yeti-web an IdP that itself federates to another IdP inserts a pointless hop and -leaves you maintaining token signing and key rotation that a purpose-built IdP -already does better. - -Use this document when `AdminUser` with a local password is the real login. - ---- - -## What you get - -| Endpoint | Serves | -|---|---| -| `/.well-known/openid-configuration` | OIDC discovery — clients read everything else from here | -| `/oauth/discovery/keys` | JWKS: the public half of the signing key, with a `kid` | -| `/oauth/userinfo` | Claims for a bearer token | -| `/oauth/authorize`, `/oauth/token` | Unchanged; the token response gains an `id_token` when `openid` was granted | -| `/.well-known/oauth-authorization-server` | Unchanged RFC 8414 document for MCP clients, now also naming the OIDC endpoints | - -Claims in both the `id_token` and `userinfo`: - -| Claim | Source | -|---|---| -| `sub` | `AdminUser#id` — stable, never recycled | -| `name`, `preferred_username` | `AdminUser#username` | -| `email` | `AdminUser#email`, which comes off the billing contact — **nil if the admin has none** | -| `email_verified` | whether that address exists | -| `groups` | `AdminUser#roles` — this is what clients authorise on | - ---- - -## Setup - -### 1. Generate the signing key - -```sh -bundle exec rake 'oauth:oidc:generate_signing_key[/etc/yeti-web/oidc_signing_key.pem]' -``` - -2048-bit RSA, written at mode 0400. **This key mints identities**: anyone who can -read it can forge an `id_token` for any admin on every client that trusts this -issuer. Deploy it out of band (Ansible, not git) and treat it like the database -password. `config/oidc_signing_key.pem` is gitignored for the same reason. - -### 2. Enable it - -```yaml -# config/yeti_web.yml -oauth: - enabled: true - issuer: https://web.example.com - oidc: - enabled: true - signing_key_path: /etc/yeti-web/oidc_signing_key.pem -``` - -`issuer` sits on `oauth`, not on `oauth.oidc`, because it identifies the -authorization server itself: the same string is reported by -`/.well-known/oauth-authorization-server` and `/.well-known/openid-configuration` -and carried in every `id_token`. One server, one identity — a client that -discovers through either document must see the same answer. - -It must be the URL clients actually reach yeti-web on. Clients compare it against -the discovery document, against the URL they fetched it from, and against the -`iss` claim — a trailing slash is enough to break every login. It is mandatory -once `oidc.enabled`, and yeti-web refuses to boot without it (or if the key is -unreadable). With OAuth alone it may be omitted, in which case the RFC 8414 -document reports the URL the request came in on — which is what lets MCP -one-click connect work with no configuration at all. - -Run the migration if this is an upgrade: the `nonce` a client sends has to -survive between the authorization request and the token exchange, and it lives in -`gui.oauth_openid_requests`. - -### 3. Register the client - -In the admin UI: **System → Admin Access → OAuth Applications → New**. Give it a -name, a redirect URI and the `openid profile email` scopes; leave Client ID and -Client secret blank to have them generated, or set them to fixed values when the -client's config is deployed from a template. The detail page shows both -afterwards, and has a **Rotate secret** action. - -The page is root-only until a role is granted the `System/OauthApplication` -section in `config/policy_roles.yml` — it displays client secrets in cleartext, -which is how Doorkeeper stores them. - -The `redirect_uri` must match what the client sends **byte for byte**, including -any base path (`https://stats.example.com/stats/api/auth/callback`), and must be -HTTPS unless the host is a loopback address. - -MCP clients (Claude Code, Cursor) need none of this — they self-register through -`POST /oauth/register`, and appear in the same list once they have. OIDC clients -cannot: that endpoint is unauthenticated, so it grants the `mcp` scope only and -rejects a registration asking for `openid`, `profile` or `email` with -`invalid_client_metadata`. Anything that signs users in is registered here, by an -operator. - -### 4. Configure the client - -```yaml -# yeti-statistics config.yml -auth: - enabled: true - issuer: https://web.example.com - client_id: - client_secret: - redirect_url: https://stats.example.com/api/auth/callback - cookie_secret: - allowed_groups: [admin] # matched against AdminUser#roles -``` - -**Set `allowed_groups`.** Without it, any account this provider knows — every -enabled admin — can read every customer's traffic and margin. - ---- - -## Verification - -```sh -# 1. Discovery, and the issuer it claims. -curl -s https://web.example.com/.well-known/openid-configuration | jq -# issuer must equal what the client is configured with, exactly. -# Also check: jwks_uri, id_token_signing_alg_values_supported ["RS256"], -# code_challenge_methods_supported ["S256"], scopes_supported incl. "openid". - -# 2. JWKS serves a public key with a kid. -curl -s https://web.example.com/oauth/discovery/keys | jq - -# 3. MCP discovery is untouched — registration_endpoint must still be there. -curl -s https://web.example.com/.well-known/oauth-authorization-server | jq -``` - -Then run the real flow: open yeti-statistics, click **Sign in**. You should land -on the ActiveAdmin login page and come back signed in. - -When it fails, the client's error says which step broke: - -| error | cause | -|---|---| -| `provider returned no id_token: it is OAuth2, not OIDC` | `oauth.oidc.enabled` is off, or the client's registered scopes don't include `openid` | -| `id_token: issuer did not match` | `oauth.issuer` ≠ the client's `issuer` | -| `id_token: failed to verify signature` | JWKS unreachable, or the key was rotated with no overlap | -| `nonce mismatch` | the `gui.oauth_openid_requests` migration has not been run | -| `account "x" is in none of the permitted groups` | `AdminUser#roles` contains none of the client's `allowed_groups` | -| yeti-web won't boot, complaining about `oauth.issuer` / `oauth.oidc.signing_key_path` | step 1 or 2 is incomplete — the message says which | - ---- - -## Operating it - -**Key rotation.** Publish the new key in JWKS *before* signing with it, keep the -old one published until every issued token has expired, then drop the old one. -The `kid` header on each token says which key signed it. Skipping the overlap -signs everyone out. - -**Offboarding.** Setting `enabled = false` on an `AdminUser` takes effect -immediately: `OauthAccessToken#accessible?` checks the owner, so the token stops -working at `userinfo`, at introspection and at `/api/mcp` on the next request, -and `before_successful_strategy_response` blocks any new token or refresh. An -`id_token` already handed to a client stays valid until that client's own session -expires (yeti-statistics: `session_ttl`, 12h by default) — shorten it there if -immediate revocation matters. - -**Scopes.** `openid` is optional and never granted by default; a client has to -ask for it. Withholding it is also what keeps the whole OIDC layer dormant when -the feature is off. - ---- - -## Implementation notes - -Three things here are load-bearing and not obvious from the gem's README: - -- **The gem is configured unconditionally** - (`config/initializers/doorkeeper_openid_connect.rb`), even when the feature is - off. It prepends an `openid_request` association onto every Doorkeeper access - grant model, and that association reads - `Doorkeeper::OpenidConnect.configuration` while the class body is evaluated — - so leaving it unconfigured makes `OauthAccessGrant` raise at eager load, in - every deployment. What the flag actually gates is the `openid` scope and the - routes. -- **Claims declare `response: [:id_token, :user_info]`.** The gem's default is - `user_info` alone, which would leave a client that never calls - `/oauth/userinfo` — the normal case — with a token carrying nothing but `sub`. -- **Route order in `config/routes.rb` matters.** The OIDC gem's discovery routes - claim `/.well-known/oauth-authorization-server` too, and its version of that - document has no `registration_endpoint`. yeti's own route is declared first so - MCP clients can still self-register. - -`use_doorkeeper` also skips Doorkeeper's built-in `:applications` controller. -That UI was already unreachable (`admin_authenticator` answers 403), and leaving -it mounted claimed the `oauth_application(s)` route-helper names that the -ActiveAdmin page needs for its own links. - -**RP-initiated logout** (`end_session_endpoint`) is not implemented. Clients -default to dropping their own session and leaving yeti-web's alone, which is the -right default: a "sign out" button in a stats dashboard should not sign the user -out of everything on the issuer. diff --git a/app/admin/system/oauth_applications.rb b/app/admin/system/oauth_applications.rb index 7fe346ff7..2d74e7f00 100644 --- a/app/admin/system/oauth_applications.rb +++ b/app/admin/system/oauth_applications.rb @@ -1,38 +1,11 @@ # frozen_string_literal: true -# Registered OAuth/OIDC clients — the things that sign users in through yeti or -# call its API: yeti-statistics, Grafana, an internal tool. This page is the only -# way to register one. -# -# MCP clients (Claude Code, Cursor, ...) do not need this page: they self-register -# through POST /oauth/register (RFC 7591) as public PKCE clients. They still show -# up in the list once they have. That endpoint is unauthenticated, so it hands out -# the MCP scope and nothing else — the OIDC scopes carry the admin's email and -# roles and can only be granted here. See SELF_REGISTRABLE_SCOPES in -# app/controllers/oauth/registrations_controller.rb. -# -# Access is role-gated by OauthApplicationPolicy through the -# "System/OauthApplication" section of config/policy_roles.yml. A role with no -# such section falls back to its "Default" section, and the shipped template -# (config/policy_roles.yml.distr) gives `user` a permissive Default — so `user` -# manages clients out of the box, which is intended: it is an administrator role -# here. `reporter` reaches the page read-only by the same fallback. -# -# Read access matters more here than on most pages: the show page displays the -# client secret in cleartext — which is how Doorkeeper stores it, and which an -# operator configuring a client has to be able to read back. A deployment that -# wants either role kept off the page adds an explicit -# "System/OauthApplication" section for it. ActiveAdmin.register OauthApplication do menu parent: ['System', 'Admin Access'], label: 'OAuth Applications', priority: 98 config.batch_actions = false config.sort_order = 'created_at_desc' - # uid and secret may only be chosen at registration time — letting them change - # afterwards would silently break a client that is already using them, and the - # form doesn't offer them on edit. Permitting them only on create means a - # hand-crafted POST can't do it either. permit_params do permitted = %i[name redirect_uri confidential] permitted += %i[uid secret] if params[:action] == 'create' @@ -61,8 +34,6 @@ row :id row :name row('Client ID', &:uid) - # Cleartext, deliberately: this is the only place an operator can recover - # it, and Doorkeeper is storing it in cleartext regardless. row('Client secret', &:plaintext_secret) row :scopes row('Confidential', &:confidential?) @@ -72,7 +43,6 @@ end panel 'Active tokens' do - # Deleting the client deletes these with it (dependent: :delete_all). para "#{oauth_application.access_tokens.where(revoked_at: nil).count} not revoked" end end @@ -88,10 +58,8 @@ hint: 'Where the client is sent back after sign-in. Must match what the client ' \ 'sends byte for byte, base path included, and must be HTTPS unless the host ' \ 'is a loopback address. One per line for several.' - # Before :scopes, not after — a lone boolean checkbox rendered directly - # beneath the scopes checkbox list reads as one more scope. f.input :confidential, - hint: 'On for a client that can keep a secret (a server, like yeti-statistics). ' \ + hint: 'On for a client that can keep a secret (a server-side app). ' \ 'Off for a public client that authenticates with PKCE alone.' f.input :scopes, as: :check_boxes, @@ -100,9 +68,6 @@ 'Leave empty to grant the default scopes.' if f.object.new_record? - # required: false — the model does validate presence, but Doorkeeper - # fills both in before validation when they are blank, so marking them - # required would claim the operator has to invent them. f.input :uid, label: 'Client ID', required: false, hint: 'Leave blank to generate. Set it to a fixed value when the client ' \ @@ -116,9 +81,6 @@ f.actions end - # Replaces a leaked or rotated secret without deleting the client, so existing - # tokens keep working — only the client's ability to get new ones is affected - # until its config is updated. member_action :rotate_secret, method: :put do resource.renew_secret resource.save! @@ -127,9 +89,6 @@ action_item :rotate_secret, only: :show do if authorized?(:rotate_secret) - # Url options and html options must stay separate hashes here, or `method` - # and `data` end up as query parameters and the link silently becomes a GET - # with no confirmation. link_to 'Rotate secret', { action: :rotate_secret, id: resource.id }, method: :put, diff --git a/app/policies/oauth_application_policy.rb b/app/policies/oauth_application_policy.rb index fe9aad70f..76b1713e7 100644 --- a/app/policies/oauth_application_policy.rb +++ b/app/policies/oauth_application_policy.rb @@ -1,14 +1,5 @@ # frozen_string_literal: true -# Policy for the AA "OAuth Applications" page — the registered OAuth/OIDC -# clients. Page-level access is governed by role config like any other admin -# page (config/policy_roles.yml, section "System/OauthApplication"): `read` -# controls who sees the page, `change` who can register or edit a client, -# `remove` who can delete one, and `perform` who can rotate a client secret. -# -# `read` is more sensitive here than on most pages: the show page displays the -# client secret, because Doorkeeper stores it in cleartext (hash_application_secrets -# is off) and an operator wiring up a client needs to read it back. class OauthApplicationPolicy < ::RolePolicy alias_rule :rotate_secret?, to: :perform? diff --git a/app/services/mcp/server.rb b/app/services/mcp/server.rb index 1c4a1826b..7d7029db9 100644 --- a/app/services/mcp/server.rb +++ b/app/services/mcp/server.rb @@ -57,7 +57,7 @@ def authenticate!(req) access_token = OauthAccessToken.by_token(raw_token) return nil if access_token.nil? || !access_token.accessible? # Require the `mcp` scope explicitly so future tokens issued for other - # scopes (e.g. Grafana SSO via the same OAuth server) can't call MCP. + # scopes (e.g. SSO via the same OAuth server) can't call MCP. return nil unless access_token.scopes.include?('mcp') admin_user = AdminUser.find_by(id: access_token.resource_owner_id) diff --git a/config/initializers/config.rb b/config/initializers/config.rb index 72e567cfa..6103a8eb4 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -92,27 +92,12 @@ def self.setting_files(config_root, _env) # Mounts the Doorkeeper OAuth provider (/oauth/authorize, /oauth/token, # /oauth/register, /.well-known/oauth-authorization-server). Independent - # of MCP — can be enabled on its own to power Grafana SSO or other clients. + # of MCP — can be enabled on its own to power SSO for other clients. # Block AND `enabled` key are both optional; missing → treated as false. optional(:oauth).schema do optional(:enabled).value(:bool?) - - # How yeti identifies itself as an authorization server, in both - # /.well-known/oauth-authorization-server and, when OIDC is on, - # /.well-known/openid-configuration and every id_token. One server, one - # identity — the two documents must never disagree. - # - # Optional: the RFC 8414 document falls back to the request's own base - # URL, which is what keeps MCP one-click connect zero-config. Mandatory - # once oauth.oidc.enabled, since an id_token's `iss` is compared byte for - # byte by the client and cannot be derived per request. That is enforced - # in the initializer rather than here so the message can say what to do - # about it. optional(:issuer).maybe(:string) - # Turns the OAuth provider into an OIDC provider as well: id_token, - # /.well-known/openid-configuration, JWKS and userinfo. Requires - # oauth.enabled. optional(:oidc).schema do optional(:enabled).value(:bool?) optional(:signing_key_path).maybe(:string) diff --git a/config/initializers/doorkeeper_openid_connect.rb b/config/initializers/doorkeeper_openid_connect.rb index 9db6d4919..1aa0cce87 100644 --- a/config/initializers/doorkeeper_openid_connect.rb +++ b/config/initializers/doorkeeper_openid_connect.rb @@ -1,28 +1,13 @@ # frozen_string_literal: true -# OpenID Connect layer on top of the Doorkeeper OAuth provider configured in -# doorkeeper.rb (which must run first — this file's name sorts after it, and -# Doorkeeper::OpenidConnect.configure reads Doorkeeper's ORM setting). -# -# OAuth 2 answers "may this client call the API?"; OIDC answers "who is the -# user?". Enabling this makes admin_users the login for other yeti components — -# yeti-statistics, Grafana, anything that speaks OIDC — by issuing a signed -# id_token alongside the access token. -# -# Unlike doorkeeper.rb, this block is NOT skipped when the feature is off. The -# gem prepends an `openid_request` association onto every Doorkeeper access -# grant model, and that association reads -# Doorkeeper::OpenidConnect.configuration while the class body is evaluated — -# so an unconfigured gem makes OauthAccessGrant raise MissingConfiguration the -# moment anything loads it, which in production means at eager load, in every -# deployment, whether or not OAuth is even enabled. -# -# So configuration always happens, and oauth.oidc.enabled decides only whether -# the OIDC surface is exposed: the `openid` scope (doorkeeper.rb) and the -# discovery / JWKS / userinfo routes (routes.rb). With no `openid` scope on -# offer, no client can be granted one, no id_token is ever minted, and the -# signing key below is never read — which is why it may be absent when the -# feature is off. +# Configuration always happens, even when the feature is off: the gem prepends +# an `openid_request` association onto every Doorkeeper access grant model, and +# that association reads Doorkeeper::OpenidConnect.configuration while the class +# body is evaluated — so an unconfigured gem makes OauthAccessGrant raise +# MissingConfiguration at eager load, in every deployment, whether or not OAuth +# is enabled. oauth.oidc.enabled decides only whether the OIDC surface is +# exposed: the `openid` scope (doorkeeper.rb) and the discovery / JWKS / +# userinfo routes (routes.rb). oidc_enabled = YetiConfig.oauth&.enabled && YetiConfig.oauth.oidc&.enabled oidc_issuer = nil oidc_signing_key = nil @@ -56,59 +41,39 @@ signing_algorithm :rs256 subject_types_supported [:public] - # Custom model so the table lives in the gui schema, like the other - # Doorkeeper tables. See app/models/oauth_openid_request.rb. open_id_request_class 'OauthOpenidRequest' # `sub` identifies the user forever and must never be recycled. The primary - # key qualifies; the email does not — an admin who changes address would - # come back to every client as a different person. + # key qualifies; the email does not — an admin who changes address would come + # back to every client as a different person. subject { |resource_owner, _application| resource_owner.id.to_s } # Filtering on enabled here is what stops a disabled admin's still-valid - # access token from resolving at the userinfo endpoint. Token issuance is - # blocked separately by the before_successful_strategy_response hook in - # doorkeeper.rb. + # access token from resolving at the userinfo endpoint. resource_owner_from_access_token do |access_token| AdminUser.find_by(id: access_token.resource_owner_id, enabled: true) end auth_time_from_resource_owner(&:current_sign_in_at) - # Honours prompt=login: drop the Devise session and send the admin back - # through the normal sign-in page, then on to where they were going. reauthenticate_resource_owner do |_resource_owner, return_to| store_location_for :admin_user, return_to sign_out :admin_user redirect_to new_admin_user_session_url end - # Claim generators are called with (resource_owner, scopes, access_token). - # - # `response:` decides which document a claim appears in, and it defaults to - # [:user_info] alone. Clients that read the id_token and never call - # /oauth/userinfo — the common case, and what yeti-statistics does — would - # otherwise get a token carrying nothing but `sub`, sign the user in as - # anonymous, and then fail whatever group check they run. So every claim is - # declared for both. claims do + # `response:` defaults to [:user_info] alone, so a client that reads the + # id_token and never calls /oauth/userinfo would otherwise get a token + # carrying nothing but `sub`. Every claim is declared for both. claim(:name, response: %i[id_token user_info]) { |resource_owner, _scopes| resource_owner.display_name } claim(:preferred_username, response: %i[id_token user_info]) { |resource_owner, _scopes| resource_owner.username } # AdminUser#email is not a column — it is read off the billing contact, so - # it is nil for any admin who has none. Emitting nil is the honest answer; - # clients that need an address fall back to `preferred_username`, which is - # the username and always present. + # it is nil for any admin who has none. claim(:email, response: %i[id_token user_info]) { |resource_owner, _scopes| resource_owner.email } - # Set by another admin on the billing contact, never self-asserted — so an - # address that exists is as verified as this provider can make it. claim(:email_verified, response: %i[id_token user_info]) { |resource_owner, _scopes| resource_owner.email.present? } - # The authorisation boundary. Clients match their own allowlist against - # this (yeti-statistics' `allowed_groups`); without it they fall back to - # "anyone this provider knows", which here means every enabled admin can - # read every customer's traffic and margin. - # # Tied to `openid` rather than the default scope for a non-standard claim # name, which would be `profile` — a client requesting `openid` alone would # then silently lose its only means of authorising anyone. diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index b241c25c8..c04950064 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -19,19 +19,11 @@ en: not_match_configured: "doesn't match those configured on the server." doorkeeper: - # One line per scope on the consent screen, under "This application will be - # able to:". Whatever is missing here renders as a translation_missing span, - # so every scope the server offers needs an entry. + # Shown on the consent screen; a scope with no entry here renders as a + # translation_missing span. profile / email keep the OIDC gem's wording. scopes: mcp: 'Use the MCP API as you: read CDRs and simulate routing' - # Overrides the OIDC gem's "Authenticate your account", which is true but - # incomplete: the `groups` claim rides on this scope (see - # config/initializers/doorkeeper_openid_connect.rb) and carries the admin - # roles a client authorises against. An admin approving a sign-in should - # see that they are handing over their roles, not just their identity. openid: 'Sign you in, and share your username and admin roles' - # profile / email keep the gem's wording — "View your profile information" - # and "View your email address" — which describe those claims accurately. applications: confirmations: diff --git a/config/yeti_web.yml.ci b/config/yeti_web.yml.ci index 2285bc7b2..aecf31ab8 100644 --- a/config/yeti_web.yml.ci +++ b/config/yeti_web.yml.ci @@ -43,15 +43,10 @@ sentry: oauth: enabled: true # mount Doorkeeper OAuth provider — required for the spec suite - # Deliberately NOT the host request specs run against (http://www.example.com), - # so every `iss` assertion actually discriminates between the configured issuer - # and the request's own base URL. + # Deliberately NOT the host request specs run against (http://www.example.com). issuer: https://web.example.com - # OIDC provider — required for the spec suite. The key is generated by the - # Makefile ($(oidc_test_key)) before anything boots Rails, never committed: it - # is a throwaway that signs nothing real, but a private key in the repo trips - # secret scanning all the same. + # Key generated by the Makefile ($(oidc_test_key)), never committed. oidc: enabled: true signing_key_path: config/oidc_signing_key.pem diff --git a/config/yeti_web.yml.distr b/config/yeti_web.yml.distr index ec0bdf917..3d249f60b 100644 --- a/config/yeti_web.yml.distr +++ b/config/yeti_web.yml.distr @@ -43,16 +43,13 @@ sentry: oauth: enabled: false # mount Doorkeeper OAuth provider (/oauth/*, /.well-known/oauth-authorization-server) - # The URL clients actually reach yeti-web on. Reported as `issuer` in both - # discovery documents and carried in every id_token. Optional while only - # OAuth is enabled (the metadata document then reports the URL the request - # came in on); mandatory once oidc.enabled below, where it MUST equal, byte - # for byte, the issuer configured on the client side — a trailing slash is - # enough to break every login. + # The URL clients actually reach yeti-web on. Optional for OAuth alone; + # mandatory once oidc.enabled below, where it MUST equal, byte for byte, the + # issuer configured on the client side. #issuer: https://web.example.com - # Also act as an OpenID Connect provider, so other yeti components - # (yeti-statistics, Grafana, ...) can use admin_users as their login. Adds + # Also act as an OpenID Connect provider, so other components can use + # admin_users as their login. Adds # /.well-known/openid-configuration, /oauth/discovery/keys and # /oauth/userinfo, and makes the token endpoint issue an id_token for # clients that request the `openid` scope. diff --git a/spec/features/system/oauth_applications_spec.rb b/spec/features/system/oauth_applications_spec.rb index 3663c37af..3b9dc522e 100644 --- a/spec/features/system/oauth_applications_spec.rb +++ b/spec/features/system/oauth_applications_spec.rb @@ -7,7 +7,7 @@ let!(:application) do create_oauth_application( - name: 'yeti-statistics', + name: 'stats-client', confidential: true, scopes: 'openid profile email', redirect_uri: 'https://stats.example.com/api/auth/callback' @@ -19,7 +19,7 @@ it 'lists registered clients with their client id' do visit oauth_applications_path - expect(page).to have_content('yeti-statistics') + expect(page).to have_content('stats-client') expect(page).to have_content(application.uid) end @@ -33,12 +33,12 @@ it 'registers a new client with a generated client id and secret' do visit new_oauth_application_path - fill_in 'Name', with: 'grafana' - fill_in 'Redirect uri', with: 'https://grafana.example.com/login/generic_oauth' + fill_in 'Name', with: 'dashboards' + fill_in 'Redirect uri', with: 'https://dashboards.example.com/login/generic_oauth' check 'openid' click_button 'Create' - created = OauthApplication.find_by(name: 'grafana') + created = OauthApplication.find_by(name: 'dashboards') expect(created).to be_present expect(created.uid).to be_present expect(created.plaintext_secret).to be_present diff --git a/spec/requests/oauth/openid_connect_flow_spec.rb b/spec/requests/oauth/openid_connect_flow_spec.rb index 4a8d47296..ee17b0f40 100644 --- a/spec/requests/oauth/openid_connect_flow_spec.rb +++ b/spec/requests/oauth/openid_connect_flow_spec.rb @@ -8,7 +8,7 @@ let(:admin) { create(:admin_user, :filled, roles: %w[admin noc]) } let(:application) do create_oauth_application( - name: 'yeti-statistics', + name: 'stats-client', confidential: true, scopes: 'openid profile email', redirect_uri: 'https://stats.example.com/api/auth/callback' From 3eab9da9717663cd3d615472ee07c6a1571d1f1b Mon Sep 17 00:00:00 2001 From: sdi Date: Wed, 5 Aug 2026 15:59:30 +0300 Subject: [PATCH 5/5] remove comments --- .../oauth/registrations_controller.rb | 24 ++++----------- .../oauth_authorization_server_controller.rb | 29 +++++-------------- app/models/oauth_access_token.rb | 14 +++------ config/routes.rb | 20 +++++-------- config/yeti_web.yml.development | 2 -- config/yeti_web.yml.distr | 13 --------- lib/tasks/oauth.rake | 18 ++---------- .../oauth/authorization_code_flow_spec.rb | 6 ++-- .../oauth/openid_connect_flow_spec.rb | 7 ++--- spec/requests/oauth/userinfo_spec.rb | 9 ++---- spec/requests/oauth/well_known_spec.rb | 16 ++++------ 11 files changed, 42 insertions(+), 116 deletions(-) diff --git a/app/controllers/oauth/registrations_controller.rb b/app/controllers/oauth/registrations_controller.rb index 9f4853327..6e9882a02 100644 --- a/app/controllers/oauth/registrations_controller.rb +++ b/app/controllers/oauth/registrations_controller.rb @@ -10,20 +10,10 @@ class RegistrationsController < ActionController::API # are supported. `nil` / missing = public client (treated as 'none'). SUPPORTED_AUTH_METHODS = %w[none client_secret_basic].freeze - # What a stranger may register itself for. This endpoint exists to serve MCP - # clients, so it grants the MCP scope and nothing else — in particular not - # the OIDC scopes (doorkeeper.rb), which carry the admin's email and roles - # and are meant to be registered by an operator through the admin UI (see - # app/admin/system/oauth_applications.rb). Without this the split is only a - # convention: anyone could self-register a plausibly-named client asking for - # `openid profile email` and collect an identity from the first admin who - # approves what looks like an ordinary sign-in prompt. - # - # Enforced here rather than at /oauth/authorize because this is the only - # unauthenticated way to create an application: the admin UI is authenticated - # and root-gated, Doorkeeper's own applications controller is not mounted - # (skip_controllers :applications), and there is no RFC 7592 endpoint to - # widen a client's scopes afterwards. + # What a stranger may register itself for. Not the OIDC scopes: those carry + # the admin's email and roles, and are registered by an operator through the + # admin UI. This is the only unauthenticated way to create an application, + # so the check belongs here rather than at /oauth/authorize. SELF_REGISTRABLE_SCOPES = %w[mcp].freeze def create @@ -37,10 +27,8 @@ def create }, status: 400 end - # RFC 7591 §3.2.1 also allows quietly returning a narrower `scope` than was - # asked for, but a client that is told what it may have can act on it, - # whereas one handed a silently trimmed scope only finds out later, at the - # token endpoint, with nothing to point at. + # RFC 7591 §3.2.1 allows quietly returning a narrower scope instead, but a + # client handed a silently trimmed one only finds out at the token endpoint. requested_scopes = params['scope'].to_s.split unsupported_scopes = requested_scopes - SELF_REGISTRABLE_SCOPES if unsupported_scopes.any? diff --git a/app/controllers/well_known/oauth_authorization_server_controller.rb b/app/controllers/well_known/oauth_authorization_server_controller.rb index 488c5f3bc..e6a49f487 100644 --- a/app/controllers/well_known/oauth_authorization_server_controller.rb +++ b/app/controllers/well_known/oauth_authorization_server_controller.rb @@ -24,31 +24,18 @@ def show private - # The one identity this authorization server claims — the same string the - # OIDC discovery document reports and every id_token carries in `iss`, so a - # client that discovers here and validates a token there sees no mismatch. - # Emitted verbatim: clients compare it byte for byte, so normalising a - # trailing slash away here would break exactly the logins it looks like it - # fixes. - # - # Falls back to the request's own base URL when unset, which is the MCP-only - # deployment: oauth.enabled with no oidc block, nothing to compare against, - # zero config. oauth.issuer is mandatory once OIDC is on — see - # config/initializers/doorkeeper_openid_connect.rb. - # - # The endpoints below deliberately stay on request.base_url: the OIDC gem - # builds its own discovery document from Rails URL helpers, i.e. from the - # request, and two documents advertising different hosts for /oauth/authorize - # would be a worse failure than the one this avoids. If a proxy really does - # rewrite host or scheme, fix it with X-Forwarded-* / default_url_options. + # Emitted verbatim — clients compare `iss` byte for byte, so normalising a + # trailing slash here would break the logins it looks like it fixes. The + # endpoints stay on request.base_url because the OIDC gem builds its own + # discovery document from the request; two documents naming different hosts + # for /oauth/authorize would be worse than the mismatch this avoids. def issuer YetiConfig.oauth&.issuer.presence || request.base_url end - # When yeti is also an OIDC provider, say so here. A client that reads only - # this document (it is served at a path the OIDC gem would otherwise claim - # — see config/routes.rb) should not conclude there is no OIDC on offer. - # The authoritative OIDC document is /.well-known/openid-configuration. + # This document is served at a path the OIDC gem would otherwise claim (see + # config/routes.rb), so a client reading only it must not conclude there is + # no OIDC on offer. def oidc_metadata return {} unless YetiConfig.oauth&.oidc&.enabled diff --git a/app/models/oauth_access_token.rb b/app/models/oauth_access_token.rb index b4698051b..0e5aef071 100644 --- a/app/models/oauth_access_token.rb +++ b/app/models/oauth_access_token.rb @@ -32,18 +32,12 @@ class OauthAccessToken < ApplicationRecord include ::Doorkeeper::Orm::ActiveRecord::Mixins::AccessToken self.table_name = 'gui.oauth_access_tokens' - # resource_owner_id is the AdminUser id (single-tenant config — Doorkeeper - # supports polymorphic owners but we don't use it). Explicit belongs_to so - # the AA index page can eager-load with `.includes(:resource_owner)` and - # avoid an N+1 in the Owner column for root admins. + # Explicit belongs_to so the AA index page can eager-load the Owner column. belongs_to :resource_owner, class_name: 'AdminUser', foreign_key: :resource_owner_id, optional: true - # A token is only as valid as the admin behind it. Doorkeeper's own answer is - # "not expired and not revoked", which leaves a disabled admin's unexpired - # token working until it runs out — up to an hour of access after offboarding, - # on every endpoint that authorizes with it (/oauth/userinfo, introspection, - # /api/mcp). Checking the owner here revokes that access immediately, in one - # place, instead of once per endpoint. + # Doorkeeper's own answer is "not expired and not revoked", which leaves a + # disabled admin's token working until it expires. Checking the owner here + # cuts access off immediately, on every endpoint at once. def accessible? return super if resource_owner_id.nil? diff --git a/config/routes.rb b/config/routes.rb index 11e251584..ef1f41419 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,26 +23,20 @@ def dasherized_resource(name, options = {}, &block) # Doorkeeper OAuth provider + RFC 8414 metadata + RFC 7591 dynamic client # registration. Gated on YetiConfig.oauth.enabled so the surface is opt-in. if YetiConfig.oauth&.enabled - # skip_controllers :applications — Doorkeeper's built-in client-management UI - # is deliberately unavailable (doorkeeper.rb's admin_authenticator answers - # 403), and clients are managed through the ActiveAdmin "OAuth Applications" - # page instead. Leaving it mounted would also claim the oauth_application(s) - # route helper names, which that AA page needs for its own links. + # skip_controllers :applications — clients are managed through the AA "OAuth + # Applications" page, and leaving Doorkeeper's own UI mounted would claim the + # oauth_application(s) route helper names that page needs. use_doorkeeper do skip_controllers :applications end - # Must stay above use_doorkeeper_openid_connect: the OIDC gem's discovery - # routes claim /.well-known/oauth-authorization-server too, and its version - # of that document has no registration_endpoint (it only advertises one for - # its own dynamic registration, which we don't use). First route wins, so - # this ordering is what keeps MCP clients able to self-register. + # Must stay above use_doorkeeper_openid_connect: the OIDC gem claims + # /.well-known/oauth-authorization-server too, and its version of that + # document has no registration_endpoint. First route wins. get '/.well-known/oauth-authorization-server', to: 'well_known/oauth_authorization_server#show' post '/oauth/register', to: 'oauth/registrations#create' # /.well-known/openid-configuration, /oauth/discovery/keys (JWKS) and - # /oauth/userinfo. Guarded on the same condition as the initializer — - # this helper reads Doorkeeper::OpenidConnect.configuration while routes - # are drawn and raises MissingConfiguration if it was never configured. + # /oauth/userinfo. use_doorkeeper_openid_connect if YetiConfig.oauth.oidc&.enabled end diff --git a/config/yeti_web.yml.development b/config/yeti_web.yml.development index da11c3e7c..a12e446fe 100644 --- a/config/yeti_web.yml.development +++ b/config/yeti_web.yml.development @@ -43,8 +43,6 @@ sentry: oauth: enabled: false # mount Doorkeeper OAuth provider (/oauth/*, /.well-known/oauth-authorization-server) - # OpenID Connect provider on top of it — see yeti_web.yml.distr for what each - # key means. `rake oauth:oidc:generate_signing_key` writes the key. #issuer: http://127.0.0.1:3000 #oidc: # enabled: true diff --git a/config/yeti_web.yml.distr b/config/yeti_web.yml.distr index 3d249f60b..b0c169321 100644 --- a/config/yeti_web.yml.distr +++ b/config/yeti_web.yml.distr @@ -43,22 +43,9 @@ sentry: oauth: enabled: false # mount Doorkeeper OAuth provider (/oauth/*, /.well-known/oauth-authorization-server) - # The URL clients actually reach yeti-web on. Optional for OAuth alone; - # mandatory once oidc.enabled below, where it MUST equal, byte for byte, the - # issuer configured on the client side. #issuer: https://web.example.com - - # Also act as an OpenID Connect provider, so other components can use - # admin_users as their login. Adds - # /.well-known/openid-configuration, /oauth/discovery/keys and - # /oauth/userinfo, and makes the token endpoint issue an id_token for - # clients that request the `openid` scope. #oidc: # enabled: true - # # RS256 private key that signs the id_token. Generate with - # # bundle exec rake oauth:oidc:generate_signing_key[/etc/yeti-web/oidc_signing_key.pem] - # # and deploy it out of band: whoever can read it can mint an identity for - # # any admin. # signing_key_path: /etc/yeti-web/oidc_signing_key.pem mcp: diff --git a/lib/tasks/oauth.rake b/lib/tasks/oauth.rake index eb052c5bf..9d2d4cd52 100644 --- a/lib/tasks/oauth.rake +++ b/lib/tasks/oauth.rake @@ -1,17 +1,7 @@ # frozen_string_literal: true namespace :oauth do - # Clients are registered in the admin UI (System → Admin Access → OAuth - # Applications), not from here. What is left is the one thing the UI cannot - # do: put a private key on the server's filesystem. namespace :oidc do - # The id_token is signed with this key, so whoever can read it can mint an - # identity for any admin on any client that trusts this issuer. Treat it - # like the database password: 0400, deployed out of band, never in git. - # - # Rotation: publish the new key in JWKS before signing with it, keep the old - # one published until every issued token has expired, then drop the old one. - # # Usage: # rake 'oauth:oidc:generate_signing_key[/etc/yeti-web/oidc_signing_key.pem]' desc 'Generate the RSA key that signs id_tokens' @@ -20,11 +10,9 @@ namespace :oauth do key = OpenSSL::PKey::RSA.new(2048) - # Created at 0400 rather than written and then chmod'ed: File.write would - # apply the process umask, typically leaving the key world-readable until - # the next statement ran — a window another user on the host can read it - # in. O_EXCL makes the refusal-to-overwrite atomic too, where a preceding - # File.exist? check would be a TOCTOU. + # Created at 0400 rather than written then chmod'ed: File.write applies the + # umask, leaving the key world-readable until the next statement. O_EXCL + # makes the refusal-to-overwrite atomic instead of a TOCTOU. begin File.open(path, File::WRONLY | File::CREAT | File::EXCL, 0o400) do |f| f.write(key.to_pem) diff --git a/spec/requests/oauth/authorization_code_flow_spec.rb b/spec/requests/oauth/authorization_code_flow_spec.rb index 59247024a..1874f0c5b 100644 --- a/spec/requests/oauth/authorization_code_flow_spec.rb +++ b/spec/requests/oauth/authorization_code_flow_spec.rb @@ -44,10 +44,8 @@ def create_authorization_code expect(response).to have_http_status(:success) end - # The consent screen renders one line per scope from doorkeeper.scopes.*, and - # a missing key renders a "translation missing" span rather than failing — so - # a scope added without a description reaches admins as noise, on the one - # screen where they decide what to hand over. + # A missing doorkeeper.scopes.* key renders a "translation missing" span + # rather than failing, on the screen where admins decide what to hand over. it 'describes every configured scope in plain English' do Doorkeeper.config.scopes.each do |scope| expect(I18n.exists?("doorkeeper.scopes.#{scope}")) diff --git a/spec/requests/oauth/openid_connect_flow_spec.rb b/spec/requests/oauth/openid_connect_flow_spec.rb index ee17b0f40..f490e7372 100644 --- a/spec/requests/oauth/openid_connect_flow_spec.rb +++ b/spec/requests/oauth/openid_connect_flow_spec.rb @@ -79,10 +79,9 @@ def decode_id_token(id_token) expect(claims['sub']).to eq(admin.id.to_s) end - # These are in the id_token only because every claim declares - # response: [:id_token, :user_info]. The gem's default is user_info alone, - # which would leave a client that never calls /oauth/userinfo — the normal - # case — with a token carrying nothing but sub. + # In the id_token only because every claim declares response: both. The gem + # defaults to user_info alone, which leaves a client that never calls + # /oauth/userinfo with a token carrying nothing but sub. it 'carries the profile claims in the id_token itself' do expect(claims['name']).to eq(admin.username) expect(claims['preferred_username']).to eq(admin.username) diff --git a/spec/requests/oauth/userinfo_spec.rb b/spec/requests/oauth/userinfo_spec.rb index aa4f8ddaf..b7627b925 100644 --- a/spec/requests/oauth/userinfo_spec.rb +++ b/spec/requests/oauth/userinfo_spec.rb @@ -22,12 +22,9 @@ expect(payload['groups']).to match_array(%w[admin]) end - # Each claim is gated on a scope, so a client gets only what it was granted. - # None of the claim definitions passes `scope:` except `groups` — the gem - # derives the rest from Claims::Claim::STANDARD_CLAIMS (`name` and - # `preferred_username` → profile, `email`/`email_verified` → email) and - # ClaimsBuilder.generate filters on it. That is a gem default carrying a - # privacy guarantee, so pin it here rather than trust it to survive an upgrade. + # Only `groups` declares `scope:`; the gem derives the rest from + # Claims::Claim::STANDARD_CLAIMS. That default carries a privacy guarantee, so + # pin it rather than trust it to survive a gem upgrade. context 'with a token granted openid alone' do let(:token) { issue_access_token(admin: admin, application: application, scopes: 'openid') } diff --git a/spec/requests/oauth/well_known_spec.rb b/spec/requests/oauth/well_known_spec.rb index aad87cd9e..e7a0886d4 100644 --- a/spec/requests/oauth/well_known_spec.rb +++ b/spec/requests/oauth/well_known_spec.rb @@ -42,11 +42,9 @@ expect(payload['id_token_signing_alg_values_supported']).to include('RS256') end - # Since this document also advertises jwks_uri and userinfo_endpoint, a client - # may discover here and then validate an id_token minted against the OIDC - # document's issuer. One authorization server, one identity — if these two ever - # disagree, every such login fails with an issuer mismatch while - # /.well-known/openid-configuration looks perfectly healthy. + # This document advertises jwks_uri and userinfo_endpoint, so a client may + # discover here and validate an id_token minted against the OIDC document's + # issuer. If the two disagree, every such login fails on issuer mismatch. it 'claims the same issuer as the OIDC discovery document' do subject rfc8414 = JSON.parse(response.body) @@ -55,15 +53,13 @@ openid = JSON.parse(response.body) expect(rfc8414['issuer']).to eq(openid['issuer']) - # ...and it is the configured issuer, not the host the request came in on — - # yeti_web.yml.ci sets the two to different values on purpose. + # yeti_web.yml.ci sets issuer and the request host to different values. expect(rfc8414['issuer']).to eq(YetiConfig.oauth.issuer) expect(rfc8414['issuer']).not_to eq('http://www.example.com') end - # The MCP-only deployment: oauth.enabled with no issuer configured. Nothing to - # compare an id_token against, so the request's own base URL is the honest - # answer — and it keeps one-click connect working with zero config. + # The MCP-only deployment: oauth.enabled with no issuer configured, so nothing + # to compare an id_token against and one-click connect stays zero-config. context 'when oauth.issuer is not configured' do before { allow(YetiConfig.oauth).to receive(:issuer).and_return(nil) }