diff --git a/Installation.md b/Installation.md
index 2739c08b4..17fac6fbe 100644
--- a/Installation.md
+++ b/Installation.md
@@ -98,6 +98,51 @@ service postgresql restart
```
after to implement changes
+#### Alternative: run PostgreSQL in a container
+
+The committed `config/database.yml` expects a local PostgreSQL with a `tmc` superuser
+reachable over the Unix socket, which is how the project and CI run — do not change those
+defaults. If you only need to run the test suite and would rather not create that role or
+edit `pg_hba.conf` on your machine (for example on a shared host where you are not root),
+run a throwaway PostgreSQL in Docker instead and point just the test environment at it.
+
+`config/database.yml` ends by ERB-including `config/database.local.yml` if it exists, so
+that file can override any environment. It is gitignored — keep it that way, it is a
+machine-local override and must never be committed.
+
+Start the container (the version should match production; 14 at the time of writing):
+
+```bash
+docker run -d --name tmc-test-pg -p 127.0.0.1:5433:5432 \
+ -e POSTGRES_USER=tmc -e POSTGRES_PASSWORD=tmc -e POSTGRES_DB=tmc-test postgres:14
+```
+
+Create `config/database.local.yml`:
+
+```yaml
+test:
+ adapter: postgresql
+ username: tmc
+ password: tmc
+ database: tmc-test
+ host: 127.0.0.1
+ port: 5433
+ pool: 25
+```
+
+Load the schema and run specs:
+
+```bash
+RAILS_ENV=test bundle exec rake db:schema:load
+RAILS_ENV=test bundle exec rspec spec/services
+```
+
+A non-default port (5433 above) keeps the container from colliding with a system
+PostgreSQL on 5432. Note this covers the test environment only — the sandbox-backed
+integration specs and the dev server still want the full local setup described above.
+Remove the container with `docker rm -f tmc-test-pg` and delete
+`config/database.local.yml` when you are done.
+
### TMC-server installation
#### Clone the TMC repository
diff --git a/app/controllers/api/v8/apidocs_controller.rb b/app/controllers/api/v8/apidocs_controller.rb
index ff3dfc918..f8b984310 100644
--- a/app/controllers/api/v8/apidocs_controller.rb
+++ b/app/controllers/api/v8/apidocs_controller.rb
@@ -62,9 +62,9 @@ class ApidocsController < ActionController::Base
key :required, true
key :type, :integer
end
- parameter :path_user_email do
- key :name, :user_email
- key :in, :path
+ parameter :query_user_email do
+ key :name, :email
+ key :in, :query
key :description, "User's email"
key :required, true
key :type, :string
diff --git a/app/controllers/api/v8/base_controller.rb b/app/controllers/api/v8/base_controller.rb
index a83f1c1c8..ff2818873 100644
--- a/app/controllers/api/v8/base_controller.rb
+++ b/app/controllers/api/v8/base_controller.rb
@@ -34,18 +34,113 @@ def present(hash)
end
end
+ # The exact UUID format the User model enforces on courses_mooc_fi_user_id (see
+ # app/models/user.rb). Duplicated here so the introspected subject is shape-checked before
+ # any lookup or the update_column backfill, which bypasses that load-bearing model validation.
+ COURSES_MOOC_FI_USER_ID_FORMAT = /\A\h{8}-\h{4}-\h{4}-\h{4}-\h{12}\z/
+
private
def authenticate_user!
return @current_user if @current_user
if doorkeeper_token
@current_user ||= User.find_by(id: doorkeeper_token.resource_owner_id)
raise 'Invalid token' unless @current_user
+ elsif Rails.configuration.x.accept_courses_mooc_fi_tokens && (bearer = bearer_token).present?
+ @current_user ||= user_from_courses_mooc_fi_token(bearer)
end
@current_user ||= user_from_session || Guest.new
end
attr_reader :current_user
+ # Additive, feature-flagged auth path (Rails.configuration.x.accept_courses_mooc_fi_tokens,
+ # default off). Only reached when there is no native Doorkeeper token. Treats the bearer as
+ # a courses.mooc.fi (secret-project-331) OAuth token, validates it via RFC 7662
+ # introspection, and maps it to a local user. Fails closed to nil (caller resolves Guest)
+ # on any problem; never raises.
+ def user_from_courses_mooc_fi_token(token)
+ result = CoursesMoocFiTokenIntrospector.introspect(token)
+ return nil unless result
+
+ unless result.scope?('exercise-services')
+ Rails.logger.warn('courses.mooc.fi token rejected: missing exercise-services scope')
+ return nil
+ end
+
+ # The subject is about to be used as courses_mooc_fi_user_id, both for the find_by below
+ # and (on a cache miss) for the update_column backfill, which bypasses the model's
+ # load-bearing UUID-format validation. Shape-check it once here so a malformed subject can
+ # neither be looked up nor persisted. Fail closed on mismatch.
+ unless COURSES_MOOC_FI_USER_ID_FORMAT.match?(result.sub)
+ Rails.logger.warn('courses.mooc.fi token rejected: subject is not a valid UUID')
+ return nil
+ end
+
+ user = User.find_by(courses_mooc_fi_user_id: result.sub)
+ user ||= backfill_from_upstream_id(result)
+ return nil unless user
+
+ # Decision 5 (widened 2026-07-23): introspected tokens must never resolve to an elevated
+ # user. Originally admins-only; now also blocks anyone holding any teachership or
+ # assistantship, because those grant real CanCan abilities (manage exercises/deadlines,
+ # read others' submissions). Elevated users keep using native tmc tokens; fail closed to
+ # Guest here.
+ reason = elevated_user_reason(user)
+ if reason
+ Rails.logger.warn("courses.mooc.fi token resolved to #{reason} user #{user.id}; refusing introspected auth (elevated users must use native tmc tokens)")
+ return nil
+ end
+
+ user
+ rescue => e
+ Rails.logger.warn("courses.mooc.fi token authentication error: #{e.class}: #{e.message}")
+ nil
+ end
+
+ # nil when the user holds no elevated role; otherwise a short reason naming the highest
+ # concern (administrator > teacher > assistant), used only for the warn log. Uses efficient
+ # existence checks rather than loading and iterating every organization/course.
+ def elevated_user_reason(user)
+ return 'administrator' if user.administrator?
+ return 'teacher' if Teachership.exists?(user_id: user.id)
+ return 'assistant' if Assistantship.exists?(user_id: user.id)
+ nil
+ end
+
+ # No user is mapped to this token's subject yet, but the introspection response carries the
+ # TMC integer id (upstream_id). Look the user up by it and backfill the UUID so later
+ # requests resolve directly. Guarded against the unique-index race and against clobbering a
+ # user already bound to a different subject.
+ def backfill_from_upstream_id(result)
+ upstream_id = result.upstream_id
+ return nil if upstream_id.blank?
+
+ user = User.find_by(id: upstream_id)
+ return nil unless user
+
+ if user.courses_mooc_fi_user_id.blank?
+ begin
+ user.update_column(:courses_mooc_fi_user_id, result.sub)
+ rescue ActiveRecord::RecordNotUnique
+ # Another request backfilled the same subject first. Trust the authoritative mapping.
+ user = User.find_by(courses_mooc_fi_user_id: result.sub)
+ end
+ elsif user.courses_mooc_fi_user_id != result.sub
+ # upstream_id points at a user already bound to a different subject. Do not override.
+ Rails.logger.warn("courses.mooc.fi upstream_id #{upstream_id} maps to user #{user.id} already bound to a different courses_mooc_fi_user_id; refusing")
+ return nil
+ end
+
+ user
+ end
+
+ def bearer_token
+ auth = request.authorization
+ return nil unless auth
+ match = auth.match(/\ABearer[ ]+(.+)\z/i)
+ match && match[1]
+ end
+
def errors_json(messages)
{ errors: [*messages] }
end
diff --git a/app/controllers/api/v8/core/exercises/details_controller.rb b/app/controllers/api/v8/core/exercises/details_controller.rb
index 736642c1d..784edfa4d 100644
--- a/app/controllers/api/v8/core/exercises/details_controller.rb
+++ b/app/controllers/api/v8/core/exercises/details_controller.rb
@@ -16,14 +16,12 @@ class DetailsController < Api::V8::BaseController
parameter do
key :in, 'query'
key :name, 'ids'
- schema do
- key :type, :array
- items do
- key :type, :integer
- end
- end
- key :type, :array
key :description, 'Exercise Ids'
+ key :type, :array
+ key :collectionFormat, 'csv'
+ items do
+ key :type, :integer
+ end
end
response 200 do
key :description, 'Exercises in json'
diff --git a/app/controllers/api/v8/exercises/users/submissions_controller.rb b/app/controllers/api/v8/exercises/users/submissions_controller.rb
index f2f436ed4..391926c2a 100644
--- a/app/controllers/api/v8/exercises/users/submissions_controller.rb
+++ b/app/controllers/api/v8/exercises/users/submissions_controller.rb
@@ -7,7 +7,7 @@ module Users
class SubmissionsController < Api::V8::BaseController
include Swagger::Blocks
- swagger_path 'api/v8/exercises/{exercise_id}/users/{user_id}/submissions' do
+ swagger_path '/api/v8/exercises/{exercise_id}/users/{user_id}/submissions' do
operation :get do
key :description, 'Returns the submissions visible to the user in a json format'
key :operationId, 'findUsersSubmissionsForExerciseById'
@@ -33,7 +33,7 @@ class SubmissionsController < Api::V8::BaseController
end
end
- swagger_path 'api/v8/exercises/{exercise_id}/users/current/submissions' do
+ swagger_path '/api/v8/exercises/{exercise_id}/users/current/submissions' do
operation :get do
key :description, "Returns the current user's submissions for the exercise in a json format. The exercise is searched by id."
key :operationId, 'findUsersOwnSubmissionsForExerciseById'
diff --git a/app/controllers/api/v8/users_controller.rb b/app/controllers/api/v8/users_controller.rb
index 097b7a4ff..290c10a64 100644
--- a/app/controllers/api/v8/users_controller.rb
+++ b/app/controllers/api/v8/users_controller.rb
@@ -62,7 +62,7 @@ class UsersController < Api::V8::BaseController
key :operationId, 'setPasswordManagedByCoursesMoocFi'
key :produces, ['application/json']
key :tags, ['user']
- parameter '$ref': '#/parameters/user_id'
+ parameter '$ref': '#/parameters/path_user_id'
response 403, '$ref': '#/responses/error'
response 404, '$ref': '#/responses/error'
response 200 do
@@ -76,18 +76,17 @@ class UsersController < Api::V8::BaseController
end
end
- swagger_path '/api/v8/users/get_user_with_email?email={email}' do
+ swagger_path '/api/v8/users/get_user_with_email' do
operation :get do
key :description, "Returns the user's id as upstream_id, user's courses.mooc.fi-id as id, email, first name and last name by user email"
key :operationId, 'getUserInformationByEmail'
key :produces, ['application/json']
key :tags, ['user']
- parameter '$ref': '#/parameters/user_email'
+ parameter '$ref': '#/parameters/query_user_email'
response 403, '$ref': '#/responses/error'
response 404, '$ref': '#/responses/error'
response 200 do
key :description, "User's courses.mooc.fi-id as id, email, first name, last name and id as upstream_id as json"
- key :content, 'application/json'
schema do
key :title, :user
key :required, [:user]
diff --git a/app/models/user.rb b/app/models/user.rb
index 85e18e3d3..f3ceb9f06 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1,5 +1,7 @@
# frozen_string_literal: true
+require 'app_secrets'
+
class User < ApplicationRecord
include Comparable
include Gravtastic
@@ -206,7 +208,7 @@ def authenticate_via_courses_mooc_fi(submitted_password)
response = conn.post(auth_url) do |req|
req.headers['Content-Type'] = 'application/json'
req.headers['Accept'] = 'application/json'
- req.headers['Authorization'] = Rails.application.secrets.tmc_server_secret_for_communicating_to_secret_project
+ req.headers['Authorization'] = AppSecrets.tmc_server_secret_for_communicating_to_secret_project
req.body = {
user_id: courses_mooc_fi_user_id,
@@ -256,7 +258,7 @@ def update_password_via_courses_mooc_fi(old_password, new_password)
response = conn.post(update_url) do |req|
req.headers['Content-Type'] = 'application/json'
req.headers['Accept'] = 'application/json'
- req.headers['Authorization'] = Rails.application.secrets.tmc_server_secret_for_communicating_to_secret_project
+ req.headers['Authorization'] = AppSecrets.tmc_server_secret_for_communicating_to_secret_project
req.body = {
user_id: self.courses_mooc_fi_user_id,
@@ -314,7 +316,7 @@ def post_new_user_to_courses_mooc_fi(password)
response = conn.post(create_url) do |req|
req.headers['Content-Type'] = 'application/json'
req.headers['Accept'] = 'application/json'
- req.headers['Authorization'] = Rails.application.secrets.tmc_server_secret_for_communicating_to_secret_project
+ req.headers['Authorization'] = AppSecrets.tmc_server_secret_for_communicating_to_secret_project
req.body = {
upstream_id: id,
diff --git a/app/services/courses_mooc_fi_token_introspector.rb b/app/services/courses_mooc_fi_token_introspector.rb
new file mode 100644
index 000000000..d4a70ff9a
--- /dev/null
+++ b/app/services/courses_mooc_fi_token_introspector.rb
@@ -0,0 +1,214 @@
+# frozen_string_literal: true
+
+require 'app_secrets'
+require 'digest'
+
+# Validates courses.mooc.fi (secret-project-331) OAuth2 access tokens against the
+# provider's RFC 7662 token introspection endpoint.
+#
+# This is the tmc-server side of the auth migration: newer tmc-vscode versions log
+# in once at courses.mooc.fi and send that bearer token to both backends. tmc-server
+# cannot validate such a token locally (it is opaque and lives in the sp331
+# database), so it asks the provider whether the token is active and who it belongs
+# to.
+#
+# Fails closed: any error at all (missing config, network failure, non-200 status,
+# malformed JSON, an inactive token, a response without a subject) yields nil, so
+# the caller falls back to Guest. Only positive results are cached, and never longer
+# than the token's own remaining lifetime nor MAX_CACHE_TTL. Failures are never
+# cached.
+class CoursesMoocFiTokenIntrospector
+ # Never trust a cached positive result longer than this, even for long-lived
+ # tokens (seconds).
+ MAX_CACHE_TTL = 300
+ CACHE_NAMESPACE = 'courses_mooc_fi_introspection'
+
+ # A validated, active introspection response. Marshalable so it round-trips
+ # through Rails.cache.
+ Result = Struct.new(:sub, :scopes, :upstream_id, :expires_at, keyword_init: true) do
+ def scope?(name)
+ scopes.include?(name)
+ end
+ end
+
+ # Returns a Result for an active token, or nil on any failure / inactive token.
+ def self.introspect(token)
+ new.introspect(token)
+ end
+
+ def introspect(token)
+ return nil if token.blank?
+
+ cached = read_cache(token)
+ return cached if cached
+
+ body = request_introspection(token)
+ return nil unless body.is_a?(Hash)
+ return nil unless body['active'] == true
+ return nil unless expected_issuer?(body)
+ return nil unless bearer_token_type?(body)
+ return nil unless client_bearer_allowed?(body)
+
+ result = build_result(body)
+ return nil if result.nil?
+
+ write_cache(token, result)
+ result
+ rescue => e
+ # Fail closed on anything unexpected (network, JSON parse, etc.).
+ Rails.logger.warn("courses.mooc.fi token introspection failed: #{e.class}: #{e.message}")
+ nil
+ end
+
+ private
+ def request_introspection(token)
+ url = SiteSetting.value('courses_mooc_fi_introspection_url')
+ client_id = AppSecrets.courses_mooc_fi_introspection_client_id
+ client_secret = AppSecrets.courses_mooc_fi_introspection_secret
+
+ # A flag-on-but-unconfigured deploy (blank URL or missing client credentials) would otherwise
+ # silently fail closed to Guest with no clue why. Emit one distinct warn so the misconfig is
+ # diagnosable; still fail closed. Distinct from the network-failure warn in #introspect.
+ if url.blank? || client_id.blank? || client_secret.blank?
+ Rails.logger.warn('courses.mooc.fi token introspection is not configured (missing URL or client credentials); refusing to introspect')
+ return nil
+ end
+
+ # Mirror the Faraday idiom used elsewhere for courses.mooc.fi calls (see
+ # User#authenticate_via_courses_mooc_fi), with tight timeouts so a hung
+ # provider can never stall an authenticated request. RFC 7662
+ # client_secret_post: client credentials go in the form body.
+ conn = Faraday.new(request: { open_timeout: 2, timeout: 5 }) do |f|
+ f.request :url_encoded
+ f.response :json
+ end
+
+ response = conn.post(url) do |req|
+ req.headers['Accept'] = 'application/json'
+ req.body = {
+ token: token,
+ client_id: client_id,
+ client_secret: client_secret
+ }
+ end
+
+ return nil if rejected_our_credentials?(response)
+ return nil unless response.status == 200
+ response.body
+ end
+
+ # A wrong-but-non-blank COURSES_MOOC_FI_INTROSPECTION_CLIENT_ID or secret is
+ # indistinguishable from a bad user token at the call site — both just fail closed to Guest —
+ # so without this it presents as "every user is logged out" with nothing naming the cause.
+ # The provider answers 401 invalid_client for rejected client credentials (RFC 7662 §2.3),
+ # separately from the 200 `active: false` it uses for an inactive token, so the two can be
+ # told apart. Log at error: this is our own misconfiguration, not a user's problem, and it
+ # affects every request rather than one.
+ def rejected_our_credentials?(response)
+ return false unless response.status == 401
+
+ error = response.body.is_a?(Hash) ? response.body['error'] : nil
+ Rails.logger.error("courses.mooc.fi rejected tmc-server's own introspection client credentials (HTTP 401, error #{error.inspect}); check COURSES_MOOC_FI_INTROSPECTION_CLIENT_ID and COURSES_MOOC_FI_INTROSPECTION_SECRET. No user can authenticate via courses.mooc.fi until this is fixed.")
+ true
+ end
+
+ # The provider stamps every active response with `iss`, its OAuth issuer identifier
+ # ("/api/v0/main-frontend/oauth"). Verify it so a response cannot be honoured as
+ # though it came from the configured provider when it did not — a misdirected
+ # courses_mooc_fi_introspection_url, or a proxy answering in its place.
+ #
+ # The expected value is derived from that same setting rather than configured separately:
+ # sp331 serves this endpoint at "/introspect", so the issuer is the configured URL
+ # minus that suffix. A second setting would only add a way for the two to disagree, and an
+ # expected-issuer knob nobody sets would verify nothing.
+ #
+ # `aud` is deliberately NOT verified: sp331 creates every access token with a null audience,
+ # so the member is never emitted (see spec/fixtures/courses_mooc_fi_introspection/). There is
+ # nothing to compare against, and requiring it would reject every token. Audience would only
+ # start to matter if tokens were minted for a specific resource server; today the
+ # exercise-services scope check at the call site is what limits what a token can be used for.
+ def expected_issuer?(body)
+ url = SiteSetting.value('courses_mooc_fi_introspection_url').to_s
+ expected = url[%r{\A(.*)/introspect/?\z}, 1]
+
+ if expected.blank?
+ Rails.logger.error("courses.mooc.fi introspection URL #{url.inspect} does not end in /introspect, so the expected issuer cannot be derived; refusing to introspect")
+ return false
+ end
+
+ return true if body['iss'] == expected
+
+ Rails.logger.warn("courses.mooc.fi token rejected: iss #{body['iss'].inspect} is not #{expected.inspect}")
+ false
+ end
+
+ # The provider mints both plain Bearer and sender-constrained (DPoP) access tokens and reports
+ # which via the introspection response's `token_type` ("Bearer" / "DPoP"). A DPoP-bound token
+ # proves nothing about whoever presents it as a plain bearer, and sp331's own client-facing API
+ # refuses one for exactly that reason (see secret-project-331
+ # server/src/domain/exercise_services/token.rs, which requires token_type == Bearer). tmc-server
+ # only ever reads tokens out of an `Authorization: Bearer` header, so mirror that rule instead of
+ # being the weaker of the two backends.
+ #
+ # A missing/unrecognised token_type is treated as not-Bearer: this class fails closed, and the
+ # provider always sends the claim for an active token.
+ def bearer_token_type?(body)
+ token_type = body['token_type']
+ return true if token_type.to_s.casecmp('bearer').zero?
+
+ Rails.logger.warn("courses.mooc.fi token rejected: token_type #{token_type.inspect} is not Bearer")
+ false
+ end
+
+ # The provider's introspection response carries a non-standard `client_bearer_allowed` member:
+ # whether the client the token was issued to may use plain Bearer tokens (sp331's own
+ # client-facing extractor refuses the token otherwise, requiring client.allows_bearer()). It is
+ # sent only to confidential callers (tmc-server always qualifies) and is OMITTED, not `false`,
+ # when withheld — so absence means "no assertion", not "allowed", and must fail closed too.
+ # Hence `== true` rather than Ruby truthiness.
+ def client_bearer_allowed?(body)
+ return true if body['client_bearer_allowed'] == true
+
+ Rails.logger.warn("courses.mooc.fi token rejected: client_bearer_allowed #{body['client_bearer_allowed'].inspect} is not true")
+ false
+ end
+
+ def build_result(body)
+ sub = body['sub']
+ return nil if sub.blank?
+
+ scopes = body['scope'].to_s.split(' ')
+ exp = body['exp']
+ expires_at = exp.is_a?(Numeric) ? Time.at(exp) : nil
+
+ Result.new(
+ sub: sub,
+ scopes: scopes,
+ upstream_id: body['upstream_id'],
+ expires_at: expires_at
+ )
+ end
+
+ def cache_key(token)
+ "#{CACHE_NAMESPACE}:#{Digest::SHA256.hexdigest(token)}"
+ end
+
+ def read_cache(token)
+ Rails.cache.read(cache_key(token))
+ end
+
+ def write_cache(token, result)
+ ttl = cache_ttl(result)
+ return if ttl.nil? || ttl <= 0
+ Rails.cache.write(cache_key(token), result, expires_in: ttl)
+ end
+
+ # TTL = min(exp - now, MAX_CACHE_TTL). A token that carries no exp is still
+ # cached, but only up to MAX_CACHE_TTL. An already-expired token is not cached.
+ def cache_ttl(result)
+ return MAX_CACHE_TTL if result.expires_at.nil?
+ remaining = (result.expires_at - Time.now).floor
+ return nil if remaining <= 0
+ [remaining, MAX_CACHE_TTL].min
+ end
+end
diff --git a/config/application.rb b/config/application.rb
index 6c61f3e3a..e9d407a15 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -34,6 +34,11 @@ class Application < Rails::Application
config.relative_url_root = SiteSetting.value('base_path')
+ # Feature flag (default off): accept courses.mooc.fi (secret-project-331) OAuth tokens on the
+ # API v8 auth path via RFC 7662 introspection. See Api::V8::BaseController#authenticate_user!
+ # and CoursesMoocFiTokenIntrospector. Additive: with this off every request path is unchanged.
+ config.x.accept_courses_mooc_fi_tokens = ENV['ACCEPT_COURSES_MOOC_FI_TOKENS'] == 'true'
+
config.middleware.insert_before 0, Rack::Cors, debug: true, logger: (-> { Rails.logger }) do
allow do
origins '*'
diff --git a/config/initializers/doorkeeper_openid_connect.rb b/config/initializers/doorkeeper_openid_connect.rb
index fab964bd0..6e85f4d0c 100644
--- a/config/initializers/doorkeeper_openid_connect.rb
+++ b/config/initializers/doorkeeper_openid_connect.rb
@@ -1,9 +1,11 @@
# frozen_string_literal: true
+require 'app_secrets'
+
Doorkeeper::OpenidConnect.configure do
issuer 'https://tmc.mooc.fi'
- signing_key Rails.application.secrets.openid_connect_signing_key
+ signing_key AppSecrets.openid_connect_signing_key
subject_types_supported [:public]
@@ -41,7 +43,7 @@
# Example implementation:
# resource_owner.id
- user_response = RestClient.get "https://courses.mooc.fi/api/v0/tmc-server/users-by-upstream-id/#{resource_owner.id}", { Authorization: Rails.application.secrets.tmc_server_secret_for_communicating_to_secret_project }
+ user_response = RestClient.get "https://courses.mooc.fi/api/v0/tmc-server/users-by-upstream-id/#{resource_owner.id}", { Authorization: AppSecrets.tmc_server_secret_for_communicating_to_secret_project }
user = JSON.parse(user_response)
diff --git a/config/secrets.yml b/config/secrets.yml
index 804d52eb3..5418a06a4 100644
--- a/config/secrets.yml
+++ b/config/secrets.yml
@@ -42,6 +42,8 @@ development:
wvTexLwtU6i38xTpGMeTsQVx
-----END PRIVATE KEY-----
tmc_server_secret_for_communicating_to_secret_project: <%= ENV["TMC_SERVER_SECRET_FOR_COMMUNICATING_TO_SECRET_PROJECT"] || "Zm9yIGxvY2FsIGRldmVsb3BtZW50IG9ubHksIGludGVudGlvbmFsbHkgcHVibGlj" %>
+ courses_mooc_fi_introspection_client_id: <%= ENV["COURSES_MOOC_FI_INTROSPECTION_CLIENT_ID"] || "tmc-server-introspection-dev" %>
+ courses_mooc_fi_introspection_secret: <%= ENV["COURSES_MOOC_FI_INTROSPECTION_SECRET"] || "for local development only, intentionally public" %>
test:
secret_key_base: fdc6fb714d1447ce6ab48e3f216e5419b567dcfbc9a713273f8f193c3f13807105136de778516770d451485a8fc31e8d58eae6259e94e2a4b53f547329bb1486
@@ -75,6 +77,14 @@ test:
wvTexLwtU6i38xTpGMeTsQVx
-----END PRIVATE KEY-----
tmc_server_secret_for_communicating_to_secret_project: <%= ENV["TMC_SERVER_SECRET_FOR_COMMUNICATING_TO_SECRET_PROJECT"] || "Zm9yIGxvY2FsIGRldmVsb3BtZW50IG9ubHksIGludGVudGlvbmFsbHkgcHVibGlj" %>
+ # Deliberately the same client as the development default. The test suite never contacts a real
+ # introspection endpoint (courses_mooc_fi_introspection_url is blank in config/site.defaults.yml,
+ # and the specs stub Faraday / the introspector outright), so this value is only ever echoed back
+ # in assertions. secret-project-331 seeds exactly one introspection client for dev/CI,
+ # `tmc-server-introspection-dev` (see its seed_oauth_clients.rs); a distinct `-test` id would be a
+ # client that exists nowhere.
+ courses_mooc_fi_introspection_client_id: <%= ENV["COURSES_MOOC_FI_INTROSPECTION_CLIENT_ID"] || "tmc-server-introspection-dev" %>
+ courses_mooc_fi_introspection_secret: <%= ENV["COURSES_MOOC_FI_INTROSPECTION_SECRET"] || "for local development only, intentionally public" %>
# Do not keep production secrets in the repository,
# instead read values from the environment.
@@ -82,3 +92,5 @@ production:
secret_key_base: <%= ENV["SECRET_KEY_BASE"] %>
openid_connect_signing_key: <%= (ENV["OPENID_CONNECT_SIGNING_KEY"] || "").dump %>
tmc_server_secret_for_communicating_to_secret_project: <%= ENV["TMC_SERVER_SECRET_FOR_COMMUNICATING_TO_SECRET_PROJECT"] %>
+ courses_mooc_fi_introspection_client_id: <%= ENV["COURSES_MOOC_FI_INTROSPECTION_CLIENT_ID"] %>
+ courses_mooc_fi_introspection_secret: <%= ENV["COURSES_MOOC_FI_INTROSPECTION_SECRET"] %>
diff --git a/config/site.defaults.yml b/config/site.defaults.yml
index 36ea5a787..ad19b8c6c 100644
--- a/config/site.defaults.yml
+++ b/config/site.defaults.yml
@@ -143,3 +143,8 @@ teacher_manual_url: http://testmycode.github.io/tmc-server/usermanual/
courses_mooc_fi_auth_url:
courses_mooc_fi_update_password_url:
courses_mooc_fi_create_user_url:
+
+# RFC 7662 introspection endpoint for validating courses.mooc.fi OAuth tokens. Only used when the
+# ACCEPT_COURSES_MOOC_FI_TOKENS flag is on. Set per deployment, e.g.
+# https://courses.mooc.fi/api/v0/main-frontend/oauth/introspect
+courses_mooc_fi_introspection_url:
diff --git a/lib/app_secrets.rb b/lib/app_secrets.rb
new file mode 100644
index 000000000..e17434896
--- /dev/null
+++ b/lib/app_secrets.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+# Application secrets, read from config/secrets.yml.
+#
+# Replaces `Rails.application.secrets`, which Rails 7.1 deprecates (it warns once per
+# process, from the first reader during boot) and Rails 7.2 removes outright.
+#
+# The deprecation points at `Rails.application.credentials` instead, but that is not a
+# drop-in here: credentials wants an encrypted config/credentials.yml.enc plus a master
+# key to decrypt it, and this app has neither. config/secrets.yml deliberately carries
+# working plaintext defaults for development and test — so a fresh checkout boots and
+# the suite runs with no key material to fetch — and reads everything else from ENV in
+# production. Moving to credentials would mean distributing a master key to every
+# deploy and every developer, and re-doing how production gets its values. That is a
+# deployment decision, not a deprecation fix.
+#
+# So keep the file and read it the way Rails still supports: `config_for` parses the
+# same per-environment, ERB-enabled YAML and returns an ActiveSupport::OrderedOptions,
+# which is the dot-accessible, nil-for-missing-key object `Rails.application.secrets`
+# already handed back. Call sites are unchanged apart from the receiver.
+#
+# Lives in lib/ rather than app/ because config/initializers/doorkeeper_openid_connect.rb
+# needs it at boot, before app/ autoloading is safe to lean on. lib/ is on $LOAD_PATH,
+# so `require 'app_secrets'` works from anywhere — the same way lib/submission_processor.rb
+# and friends are used.
+module AppSecrets
+ class << self
+ # Memoized: secrets.yml is only read at boot anyway ("be sure to restart your
+ # server when you modify this file"), and re-running ERB per lookup would mean
+ # re-reading the file on every introspection call.
+ def config
+ @config ||= Rails.application.config_for(:secrets)
+ end
+
+ # Reset the memo. For specs that need to observe a different secrets.yml; not
+ # something application code should call.
+ def reload!
+ @config = nil
+ end
+
+ delegate_missing_to :config
+ end
+end
diff --git a/spec/controllers/api/v8/apidocs_controller_spec.rb b/spec/controllers/api/v8/apidocs_controller_spec.rb
index 31bde373b..af4223839 100644
--- a/spec/controllers/api/v8/apidocs_controller_spec.rb
+++ b/spec/controllers/api/v8/apidocs_controller_spec.rb
@@ -5,12 +5,21 @@
describe Api::V8::ApidocsController, type: :controller do
it 'json provided by controller should be valid swagger' do
- pending 'Is not valid swagger at the moment'
get :index
json = response.body
schema = File.join(Rails.root, 'spec', 'resources', 'swagger-schema.json')
- validation = JSON::Validator.validate(schema, json)
- expect(validation).to be_truthy
+ errors = JSON::Validator.fully_validate(schema, json)
+ expect(errors).to be_empty, -> { "Generated apidocs are not valid Swagger 2.0:\n#{errors.join("\n")}" }
+ end
+
+ # The Swagger 2.0 JSON schema only requires a path key to start with a slash, so it
+ # cannot catch a query string smuggled into the key. Query parameters belong in
+ # `parameters`, with `in: query`.
+ it 'declares no path containing a query string' do
+ get :index
+
+ paths = JSON.parse(response.body)['paths'].keys
+ expect(paths.grep(/\?/)).to be_empty
end
end
diff --git a/spec/controllers/api/v8/base_controller_spec.rb b/spec/controllers/api/v8/base_controller_spec.rb
index 500da595d..4d3404e23 100644
--- a/spec/controllers/api/v8/base_controller_spec.rb
+++ b/spec/controllers/api/v8/base_controller_spec.rb
@@ -19,7 +19,7 @@ def index
before :each do
allow(controller).to receive(:doorkeeper_token) { token }
- get :index, format: text
+ get :index, format: :text
end
context 'when not logged in' do
diff --git a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb
index 49f5f4241..b065d666b 100644
--- a/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb
+++ b/spec/controllers/api/v8/core/exercises/submissions_controller_spec.rb
@@ -1,35 +1,96 @@
# frozen_string_literal: true
require 'spec_helper'
+require 'tmpdir'
describe Api::V8::Core::Exercises::SubmissionsController, type: :controller do
+ let(:organization) { FactoryBot.create(:accepted_organization) }
+ let(:course) { FactoryBot.create(:course, organization: organization) }
+ # returnable_exercise sets returnable_forced, so the exercise accepts submissions without a
+ # refreshed course repository behind it -- #create never looks at the exercise's files.
+ let(:exercise) { FactoryBot.create(:returnable_exercise, course: course) }
+ let(:user) { FactoryBot.create(:verified_user) }
+
+ before :each do
+ allow(controller).to receive(:doorkeeper_token) { token }
+ end
+
+ # The controller only inspects the uploaded bytes far enough to see the ZIP magic number
+ # ("PK"), so a minimal empty-archive header is enough for the accepted path and any other
+ # content for the declined one.
+ def upload(contents, filename)
+ path = File.join(Dir.mktmpdir, filename)
+ File.binwrite(path, contents)
+ Rack::Test::UploadedFile.new(path, 'application/octet-stream')
+ end
+
+ let(:zip_file) { upload("PK\x05\x06#{"\x00" * 18}", 'submission.zip') }
+ let(:text_file) { upload('this is not an archive', 'submission.txt') }
+
describe 'Creating a submission' do
describe 'as an authenticated user' do
+ let(:token) { double resource_owner_id: user.id, acceptable?: true }
+
it 'should accept submissions when the deadline is open' do
- pending('test that submission zip is accepted before deadline')
- raise
+ exercise.deadline_spec = ['1.1.2100'].to_json
+ exercise.save!
+
+ expect { post :create, params: { exercise_id: exercise.id, submission: { file: zip_file } }, format: :json }
+ .to change(Submission, :count).by(1)
+
+ expect(response).to have_http_status :ok
+ json = JSON.parse(response.body)
+ expect(json).not_to have_key('error')
+
+ submission = Submission.last
+ expect(json['submission_url']).to end_with("/api/v8/core/submissions/#{submission.id}")
+ expect(submission.user).to eq(user)
+ expect(submission.exercise_name).to eq(exercise.name)
+ expect(submission.course).to eq(course)
end
it 'should decline submissions when the deadline is closed' do
- pending('test that submission zip is declined after deadline')
- raise
+ exercise.deadline_spec = ['1.1.2000'].to_json
+ exercise.save!
+
+ expect { post :create, params: { exercise_id: exercise.id, submission: { file: zip_file } }, format: :json }
+ .not_to change(Submission, :count)
+
+ expect(response).to have_http_status :forbidden
+ expect(JSON.parse(response.body)['error']).to eq('Submissions for this exercise are no longer accepted.')
end
+ # A non-ZIP upload is reported in the response body rather than by status code: the client
+ # gets 200 with an "error" key and nothing is stored. Asserting the status alone would pass
+ # even if the magic-number check were removed, so assert the body and the count too.
it 'should decline submissions when the file is not ZIP' do
- pending('test that submission file is declined when not zip')
- raise
+ expect { post :create, params: { exercise_id: exercise.id, submission: { file: text_file } }, format: :json }
+ .not_to change(Submission, :count)
+
+ expect(response).to have_http_status :ok
+ expect(JSON.parse(response.body)['error']).to eq("The uploaded file doesn't look like a ZIP file.")
end
it 'should decline submissions when a file is not selected' do
- pending('test that submission is declined when no file is given')
- raise
+ expect { post :create, params: { exercise_id: exercise.id }, format: :json }
+ .not_to change(Submission, :count)
+
+ expect(response).to have_http_status :not_found
+ expect(JSON.parse(response.body)['error']).to eq('No ZIP file selected or failed to receive it')
end
end
describe 'as an unauthenticated user' do
+ # No Doorkeeper token and no session, so the controller resolves Guest and
+ # unauthorize_guest! rejects the request before the exercise is even loaded.
+ let(:token) { nil }
+
it 'should not allow sending submission' do
- pending('test that submission is declined when no file is given')
- raise
+ expect { post :create, params: { exercise_id: exercise.id, submission: { file: zip_file } }, format: :json }
+ .not_to change(Submission, :count)
+
+ expect(response).to have_http_status :unauthorized
+ expect(JSON.parse(response.body)['error']).to eq('Authentication required')
end
end
end
diff --git a/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb b/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb
new file mode 100644
index 000000000..f30708038
--- /dev/null
+++ b/spec/controllers/api/v8/courses_mooc_fi_token_auth_spec.rb
@@ -0,0 +1,215 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+# Exercises the additive, feature-flagged courses.mooc.fi (secret-project-331) token
+# introspection branch in Api::V8::BaseController#authenticate_user!. Mirrors the
+# UselessController pattern from base_controller_spec.rb.
+class MoocTokenUselessController < Api::V8::BaseController
+end
+
+RSpec.describe Api::V8::BaseController, type: :controller do
+ controller MoocTokenUselessController do
+ skip_authorization_check # not testing cancan here
+ def index
+ render plain: 'Success'
+ end
+ end
+
+ subject(:current_user) { assigns[:current_user] }
+
+ let(:sub) { '11111111-2222-3333-4444-555555555555' }
+ let(:bearer) { 'sp331-access-token' }
+
+ def result(scopes: ['exercise-services'], upstream_id: nil, subject_id: sub)
+ CoursesMoocFiTokenIntrospector::Result.new(
+ sub: subject_id, scopes: scopes, upstream_id: upstream_id, expires_at: Time.now + 3600
+ )
+ end
+
+ # Save/restore the flag so tests never leak global state.
+ around do |example|
+ original = Rails.configuration.x.accept_courses_mooc_fi_tokens
+ example.run
+ Rails.configuration.x.accept_courses_mooc_fi_tokens = original
+ end
+
+ before do
+ # No native Doorkeeper token in these tests unless a context overrides it.
+ allow(controller).to receive(:doorkeeper_token).and_return(nil)
+ request.headers['Authorization'] = "Bearer #{bearer}"
+ end
+
+ context 'when the flag is off (default behaviour, regression)' do
+ before { Rails.configuration.x.accept_courses_mooc_fi_tokens = false }
+
+ it 'never introspects and resolves to Guest' do
+ expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'does not even read the bearer token when the flag is off, despite a bearer header' do
+ # The flag is the first operand of the && guard, so short-circuit evaluation must skip
+ # bearer_token (and therefore introspection) entirely even though an unknown bearer is
+ # present. Pins the flag-off path's evaluation-order invariant.
+ expect(controller).not_to receive(:bearer_token)
+ expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'still authenticates a native Doorkeeper token' do
+ user = FactoryBot.create(:user)
+ allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, acceptable?: true))
+ get :index
+ expect(current_user.id).to eq(user.id)
+ end
+ end
+
+ context 'when the flag is on' do
+ before { Rails.configuration.x.accept_courses_mooc_fi_tokens = true }
+
+ it 'still prefers a native Doorkeeper token over introspection' do
+ user = FactoryBot.create(:user)
+ allow(controller).to receive(:doorkeeper_token).and_return(double(resource_owner_id: user.id, acceptable?: true))
+ expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect)
+ get :index
+ expect(current_user.id).to eq(user.id)
+ end
+
+ it 'resolves the mapped user for a valid introspected token' do
+ user = FactoryBot.create(:user, courses_mooc_fi_user_id: sub)
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).with(bearer).and_return(result)
+ get :index
+ expect(current_user.id).to eq(user.id)
+ end
+
+ it 'resolves to Guest when the exercise-services scope is missing' do
+ FactoryBot.create(:user, courses_mooc_fi_user_id: sub)
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).and_return(result(scopes: ['other-scope']))
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'resolves to Guest when introspection fails' do
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).and_return(nil)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'resolves to Guest and warns when the subject is not a valid UUID' do
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect)
+ .and_return(result(subject_id: 'not-a-uuid'))
+ expect(Rails.logger).to receive(:warn).with(/not a valid UUID/)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'resolves to Guest and warns when the mapped user is an administrator' do
+ FactoryBot.create(:admin, courses_mooc_fi_user_id: sub)
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).and_return(result)
+ expect(Rails.logger).to receive(:warn).with(/administrator/)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'resolves to Guest and warns when the mapped user holds a teachership' do
+ user = FactoryBot.create(:user, courses_mooc_fi_user_id: sub)
+ Teachership.create!(user: user, organization: FactoryBot.create(:organization))
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).and_return(result)
+ expect(Rails.logger).to receive(:warn).with(/teacher/)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'resolves to Guest and warns when the mapped user holds an assistantship' do
+ user = FactoryBot.create(:user, courses_mooc_fi_user_id: sub)
+ Assistantship.create!(user: user, course: FactoryBot.create(:course))
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).and_return(result)
+ expect(Rails.logger).to receive(:warn).with(/assistant/)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ context 'upstream_id fallback + backfill' do
+ it 'resolves via upstream_id and backfills courses_mooc_fi_user_id' do
+ user = FactoryBot.create(:user, courses_mooc_fi_user_id: nil)
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect)
+ .and_return(result(upstream_id: user.id))
+ get :index
+ expect(current_user.id).to eq(user.id)
+ expect(user.reload.courses_mooc_fi_user_id).to eq(sub)
+ end
+
+ it 'does not authenticate an administrator via the upstream_id fallback' do
+ admin = FactoryBot.create(:admin, courses_mooc_fi_user_id: nil)
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect)
+ .and_return(result(upstream_id: admin.id))
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'resolves to Guest when neither the subject nor upstream_id match a user' do
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect)
+ .and_return(result(upstream_id: 999_999))
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'refuses when upstream_id maps to a user already bound to a different subject' do
+ other_sub = '99999999-8888-7777-6666-555555555555'
+ user = FactoryBot.create(:user, courses_mooc_fi_user_id: other_sub)
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect)
+ .and_return(result(upstream_id: user.id))
+ expect(Rails.logger).to receive(:warn).with(/already bound to a different/)
+ get :index
+ expect(current_user).to be_guest
+ expect(user.reload.courses_mooc_fi_user_id).to eq(other_sub)
+ end
+
+ it 'trusts the winner of the backfill race when the unique index rejects the write' do
+ # Two concurrent requests for the same subject: this one loses the unique-index race.
+ # The rescue must re-read the authoritative mapping rather than fail to Guest.
+ winner = FactoryBot.create(:user, courses_mooc_fi_user_id: nil)
+ loser = FactoryBot.create(:user, courses_mooc_fi_user_id: nil)
+ # Simulate the racing request committing first, then our write blowing up. update_all
+ # rather than update_column, which is the stubbed method.
+ allow_any_instance_of(User).to receive(:update_column) do
+ User.where(id: winner.id).update_all(courses_mooc_fi_user_id: sub)
+ raise ActiveRecord::RecordNotUnique, 'duplicate key value violates unique constraint'
+ end
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect)
+ .and_return(result(upstream_id: loser.id))
+
+ get :index
+
+ expect(current_user.id).to eq(winner.id)
+ end
+ end
+
+ context 'bearer header parsing' do
+ it 'does not introspect when there is no Authorization header' do
+ request.headers['Authorization'] = nil
+ expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'does not introspect a non-Bearer authorization scheme' do
+ request.headers['Authorization'] = 'Basic dXNlcjpwYXNz'
+ expect(CoursesMoocFiTokenIntrospector).not_to receive(:introspect)
+ get :index
+ expect(current_user).to be_guest
+ end
+
+ it 'accepts a lower-case bearer scheme and passes the raw token through' do
+ user = FactoryBot.create(:user, courses_mooc_fi_user_id: sub)
+ request.headers['Authorization'] = "bearer #{bearer}"
+ allow(CoursesMoocFiTokenIntrospector).to receive(:introspect).with(bearer).and_return(result)
+ get :index
+ expect(current_user.id).to eq(user.id)
+ end
+ end
+ end
+end
diff --git a/spec/fixtures/courses_mooc_fi_introspection/README.md b/spec/fixtures/courses_mooc_fi_introspection/README.md
new file mode 100644
index 000000000..6d79866e9
--- /dev/null
+++ b/spec/fixtures/courses_mooc_fi_introspection/README.md
@@ -0,0 +1,18 @@
+# courses.mooc.fi introspection fixtures
+
+Hand-mirrored copies of what secret-project-331 emits from
+`POST /api/v0/main-frontend/oauth/introspect` (RFC 7662). They exist because
+`CoursesMoocFiTokenIntrospector` reads that response by member name, and every other spec in
+this repo stubs the response instead of producing it — so without these, a rename on either
+side of the wire passes CI in both repos and logs every user out in production.
+
+Upstream definition: `services/headless-lms/server/src/domain/oauth/introspect_response.rs`
+(struct `IntrospectResponse`). Its `tests::golden_serialized_shape` pins the same JSON on that
+side; `active_response.json` is a copy of the literal in that test, so the two can be diffed by
+eye. **The mirror is manual — when the upstream struct changes, update these files in the same
+change.** The pairing is only as honest as that discipline: sp331's golden test fails when its
+own output drifts, `courses_mooc_fi_introspection_contract_spec.rb` fails when these files stop
+matching what the introspector reads, and neither can observe the other repo.
+
+`aud` is deliberately absent: every access token sp331 mints is created with `audience: None`,
+so the member is never emitted. See the note in the introspector.
diff --git a/spec/fixtures/courses_mooc_fi_introspection/active_response.json b/spec/fixtures/courses_mooc_fi_introspection/active_response.json
new file mode 100644
index 000000000..86adf0419
--- /dev/null
+++ b/spec/fixtures/courses_mooc_fi_introspection/active_response.json
@@ -0,0 +1,14 @@
+{
+ "active": true,
+ "scope": "exercise-services",
+ "client_id": "tmc-server-introspection-dev",
+ "username": "11111111-2222-3333-4444-555555555555",
+ "exp": 1767225600,
+ "iat": 1767222000,
+ "sub": "11111111-2222-3333-4444-555555555555",
+ "iss": "https://courses.mooc.fi/api/v0/main-frontend/oauth",
+ "jti": "123e4567-e89b-12d3-a456-426614174000",
+ "token_type": "Bearer",
+ "upstream_id": 42,
+ "client_bearer_allowed": true
+}
diff --git a/spec/fixtures/courses_mooc_fi_introspection/inactive_response.json b/spec/fixtures/courses_mooc_fi_introspection/inactive_response.json
new file mode 100644
index 000000000..156c781ad
--- /dev/null
+++ b/spec/fixtures/courses_mooc_fi_introspection/inactive_response.json
@@ -0,0 +1,3 @@
+{
+ "active": false
+}
diff --git a/spec/services/courses_mooc_fi_introspection_contract_spec.rb b/spec/services/courses_mooc_fi_introspection_contract_spec.rb
new file mode 100644
index 000000000..317c90b58
--- /dev/null
+++ b/spec/services/courses_mooc_fi_introspection_contract_spec.rb
@@ -0,0 +1,126 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+# Members whose absence must reject the token: without them nothing has been asserted about
+# who the token belongs to or whether it may be presented as a bearer credential.
+REQUIRED_INTROSPECTION_MEMBERS = %w[active sub iss token_type client_bearer_allowed].freeze
+
+# Members the introspector reads but tolerates the absence of, each for a documented reason.
+# They still belong in the contract: a rename breaks the read just as badly, it just degrades
+# quietly instead of rejecting.
+OPTIONAL_INTROSPECTION_MEMBERS = %w[scope exp upstream_id].freeze
+
+# Cross-repo wire contract for the courses.mooc.fi (secret-project-331) introspection response.
+#
+# Every other spec touching CoursesMoocFiTokenIntrospector builds its response body inline, so
+# the member names it reads are only ever compared against themselves. This one drives the
+# introspector with a committed fixture mirrored from sp331's own output
+# (spec/fixtures/courses_mooc_fi_introspection/, see the README there for provenance) and pins
+# the members consumed, so a rename on either side fails here instead of in production.
+RSpec.describe 'courses.mooc.fi introspection response contract' do
+ def fixture(name)
+ JSON.parse(File.read(Rails.root.join('spec/fixtures/courses_mooc_fi_introspection', name)))
+ end
+
+ let(:active_response) { fixture('active_response.json') }
+ let(:inactive_response) { fixture('inactive_response.json') }
+ let(:token) { 'sp331-access-token' }
+ # The introspector derives the issuer it requires from this URL, so the fixture's `iss` has to
+ # be the one this endpoint implies.
+ let(:introspection_url) { 'https://courses.mooc.fi/api/v0/main-frontend/oauth/introspect' }
+
+ before do
+ allow(SiteSetting).to receive(:value).and_call_original
+ allow(SiteSetting).to receive(:value).with('courses_mooc_fi_introspection_url').and_return(introspection_url)
+ allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new)
+ end
+
+ def stub_provider(status:, body:)
+ response = instance_double(Faraday::Response, status: status, body: body)
+ conn = instance_double(Faraday::Connection)
+ allow(conn).to receive(:post).and_return(response)
+ allow(Faraday).to receive(:new).and_return(conn)
+ conn
+ end
+
+ it 'names every member the introspector consumes' do
+ expect(active_response.keys).to include(
+ *REQUIRED_INTROSPECTION_MEMBERS, *OPTIONAL_INTROSPECTION_MEMBERS
+ )
+ end
+
+ it 'types those members as the introspector assumes' do
+ expect(active_response['active']).to be(true)
+ expect(active_response['sub']).to be_a(String)
+ expect(active_response['scope']).to be_a(String) # space-separated, not an array
+ expect(active_response['exp']).to be_a(Numeric) # Unix seconds, not an ISO 8601 string
+ expect(active_response['iss']).to be_a(String)
+ expect(active_response['token_type']).to eq('Bearer')
+ expect(active_response['upstream_id']).to be_a(Integer)
+ expect(active_response['client_bearer_allowed']).to be(true)
+ end
+
+ # sp331 mints every access token with `audience: None`, so verifying `aud` is impossible. If
+ # this starts failing, the provider gained audience support and the introspector's recorded
+ # reasoning about not verifying it needs revisiting.
+ it 'carries no aud member' do
+ expect(active_response).not_to have_key('aud')
+ end
+
+ it 'accepts the real active response end to end' do
+ stub_provider(status: 200, body: active_response)
+
+ result = CoursesMoocFiTokenIntrospector.introspect(token)
+
+ expect(result).not_to be_nil
+ expect(result.sub).to eq(active_response['sub'])
+ expect(result).to be_scope('exercise-services')
+ expect(result.upstream_id).to eq(active_response['upstream_id'])
+ expect(result.expires_at).to eq(Time.at(active_response['exp']))
+ end
+
+ it 'rejects the real inactive response' do
+ stub_provider(status: 200, body: inactive_response)
+ expect(CoursesMoocFiTokenIntrospector.introspect(token)).to be_nil
+ end
+
+ # The negative shape carries no metadata, so nothing downstream can read a subject out of a
+ # rejected token.
+ it 'keeps the inactive response free of metadata' do
+ expect(inactive_response.keys).to eq(['active'])
+ expect(inactive_response['active']).to be(false)
+ end
+
+ REQUIRED_INTROSPECTION_MEMBERS.each do |member|
+ it "fails closed when the provider stops sending #{member}" do
+ stub_provider(status: 200, body: active_response.except(member))
+ expect(CoursesMoocFiTokenIntrospector.introspect(token)).to be_nil
+ end
+ end
+
+ describe 'members that are optional by design' do
+ # No scope member means no scopes, which closes the caller's exercise-services gate anyway —
+ # so this degrades to unauthorized rather than to unauthenticated.
+ it 'yields no scopes when scope is absent' do
+ stub_provider(status: 200, body: active_response.except('scope'))
+ result = CoursesMoocFiTokenIntrospector.introspect(token)
+ expect(result.scopes).to be_empty
+ expect(result).not_to be_scope('exercise-services')
+ end
+
+ # A token without exp is still usable; the cache just falls back to MAX_CACHE_TTL.
+ it 'yields no expiry when exp is absent' do
+ stub_provider(status: 200, body: active_response.except('exp'))
+ expect(CoursesMoocFiTokenIntrospector.introspect(token).expires_at).to be_nil
+ end
+
+ # A legitimately absent value: the token owner has no legacy TMC account to link.
+ it 'yields no upstream_id when upstream_id is absent' do
+ stub_provider(status: 200, body: active_response.except('upstream_id'))
+ result = CoursesMoocFiTokenIntrospector.introspect(token)
+ expect(result).not_to be_nil
+ expect(result.upstream_id).to be_nil
+ end
+ end
+end
diff --git a/spec/services/courses_mooc_fi_token_introspector_spec.rb b/spec/services/courses_mooc_fi_token_introspector_spec.rb
new file mode 100644
index 000000000..16b131f6c
--- /dev/null
+++ b/spec/services/courses_mooc_fi_token_introspector_spec.rb
@@ -0,0 +1,318 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe CoursesMoocFiTokenIntrospector do
+ let(:token) { 'sp331-access-token' }
+ let(:introspection_url) { 'https://courses.mooc.fi/api/v0/main-frontend/oauth/introspect' }
+ let(:sub) { '11111111-2222-3333-4444-555555555555' }
+ let(:cache) { ActiveSupport::Cache::MemoryStore.new }
+
+ # A fresh, active introspection response. exp far in the future so caching is exercised.
+ let(:active_body) do
+ {
+ 'active' => true,
+ 'sub' => sub,
+ 'scope' => 'exercise-services other-scope',
+ 'exp' => (Time.now + 3600).to_i,
+ 'upstream_id' => 42,
+ 'iss' => 'https://courses.mooc.fi/api/v0/main-frontend/oauth',
+ 'token_type' => 'Bearer',
+ 'client_bearer_allowed' => true
+ }
+ end
+
+ before do
+ allow(SiteSetting).to receive(:value).and_call_original
+ allow(SiteSetting).to receive(:value).with('courses_mooc_fi_introspection_url').and_return(introspection_url)
+ allow(Rails).to receive(:cache).and_return(cache)
+ end
+
+ # Captures what the introspector's post-block builds, so a regression in the request-building
+ # block (form body / Accept header) fails a spec instead of passing green.
+ let(:sent_headers) { {} }
+ let(:sent_body) { {} }
+
+ # Stand-in for the Faraday::Request yielded to the post block. A plain double (not an
+ # instance_double) keeps this robust across Faraday versions; it only needs #headers (a mutable
+ # hash) and #body=. #headers returns the same captured hash so header writes are observable.
+ let(:request_double) do
+ req = double('Faraday::Request')
+ allow(req).to receive(:headers).and_return(sent_headers)
+ allow(req).to receive(:body=) { |value| sent_body.replace(value) }
+ req
+ end
+
+ # Stubs Faraday so no real HTTP happens, but still RUNS the request-building block against
+ # request_double so its body/header setup is exercised. Returns the connection double.
+ def stub_faraday_response(status:, body:)
+ response = instance_double(Faraday::Response, status: status, body: body)
+ conn = instance_double(Faraday::Connection)
+ allow(conn).to receive(:post) do |_url, &blk|
+ blk&.call(request_double)
+ response
+ end
+ allow(Faraday).to receive(:new).and_return(conn)
+ conn
+ end
+
+ def stub_faraday_raise(error)
+ conn = instance_double(Faraday::Connection)
+ allow(conn).to receive(:post).and_raise(error)
+ allow(Faraday).to receive(:new).and_return(conn)
+ conn
+ end
+
+ describe '.introspect' do
+ it 'returns a result for an active token' do
+ stub_faraday_response(status: 200, body: active_body)
+ result = described_class.introspect(token)
+
+ expect(result).not_to be_nil
+ expect(result.sub).to eq(sub)
+ expect(result.scopes).to contain_exactly('exercise-services', 'other-scope')
+ expect(result).to be_scope('exercise-services')
+ expect(result.upstream_id).to eq(42)
+ end
+
+ it 'sends the token, both client credentials, and the Accept header in the request' do
+ allow(AppSecrets).to receive(:courses_mooc_fi_introspection_client_id).and_return('client-abc')
+ allow(AppSecrets).to receive(:courses_mooc_fi_introspection_secret).and_return('secret-xyz')
+ stub_faraday_response(status: 200, body: active_body)
+
+ described_class.introspect(token)
+
+ expect(sent_body).to include(
+ token: token,
+ client_id: AppSecrets.courses_mooc_fi_introspection_client_id,
+ client_secret: AppSecrets.courses_mooc_fi_introspection_secret
+ )
+ expect(sent_body[:client_id]).to eq('client-abc')
+ expect(sent_body[:client_secret]).to eq('secret-xyz')
+ expect(sent_headers['Accept']).to eq('application/json')
+ end
+
+ it 'returns nil and warns when the introspection is not configured (missing secrets)' do
+ allow(AppSecrets).to receive(:courses_mooc_fi_introspection_client_id).and_return('')
+ allow(AppSecrets).to receive(:courses_mooc_fi_introspection_secret).and_return('')
+ expect(Rails.logger).to receive(:warn).with(/not configured/)
+ expect(Faraday).not_to receive(:new)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'returns nil for a blank token without calling the provider' do
+ conn = stub_faraday_response(status: 200, body: active_body)
+ expect(described_class.introspect('')).to be_nil
+ expect(conn).not_to have_received(:post)
+ end
+
+ it 'returns nil when the introspection URL is not configured' do
+ allow(SiteSetting).to receive(:value).with('courses_mooc_fi_introspection_url').and_return(nil)
+ expect(Faraday).not_to receive(:new)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'returns nil when the token is inactive (active: false)' do
+ stub_faraday_response(status: 200, body: { 'active' => false })
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'returns nil on a non-200 status' do
+ stub_faraday_response(status: 500, body: active_body)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ # Our own credentials being rejected is a deployment fault affecting every user, not a
+ # statement about this token, so it must be distinguishable from a 200 `active: false`.
+ it 'logs distinctly at error when the provider rejects our client credentials' do
+ stub_faraday_response(status: 401, body: { 'error' => 'invalid_client' })
+ expect(Rails.logger).to receive(:error).with(/rejected tmc-server's own introspection client credentials/)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'names the misconfigured environment variables in that log' do
+ stub_faraday_response(status: 401, body: { 'error' => 'invalid_client' })
+ expect(Rails.logger).to receive(:error).with(
+ /COURSES_MOOC_FI_INTROSPECTION_CLIENT_ID.*COURSES_MOOC_FI_INTROSPECTION_SECRET/
+ )
+ described_class.introspect(token)
+ end
+
+ it 'still reports a credential rejection with an unparseable body' do
+ stub_faraday_response(status: 401, body: 'Unauthorized')
+ expect(Rails.logger).to receive(:error).with(/introspection client credentials/)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'does not log a credential rejection for an inactive token' do
+ stub_faraday_response(status: 200, body: { 'active' => false })
+ expect(Rails.logger).not_to receive(:error)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'does not log a credential rejection for an unrelated server error' do
+ stub_faraday_response(status: 500, body: active_body)
+ expect(Rails.logger).not_to receive(:error)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'does not cache a credential rejection' do
+ conn = stub_faraday_response(status: 401, body: { 'error' => 'invalid_client' })
+ allow(Rails.logger).to receive(:error)
+ described_class.introspect(token)
+ described_class.introspect(token)
+ expect(conn).to have_received(:post).twice
+ end
+
+ it 'returns nil on a network/timeout error (fails closed)' do
+ stub_faraday_raise(Faraday::TimeoutError.new('execution expired'))
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'returns nil on malformed JSON (fails closed)' do
+ stub_faraday_raise(Faraday::ParsingError.new(StandardError.new('unexpected token')))
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'returns nil when an active response has no subject' do
+ stub_faraday_response(status: 200, body: active_body.except('sub'))
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'accepts a lower-case token_type (the claim is compared case-insensitively)' do
+ stub_faraday_response(status: 200, body: active_body.merge('token_type' => 'bearer'))
+ expect(described_class.introspect(token)).not_to be_nil
+ end
+
+ it 'rejects a DPoP-bound token presented as a plain bearer' do
+ # sp331's own client-facing API is Bearer-only; a sender-constrained token proves nothing
+ # about whoever presents it here, so tmc-server must not be the weaker backend.
+ stub_faraday_response(status: 200, body: active_body.merge('token_type' => 'DPoP'))
+ expect(Rails.logger).to receive(:warn).with(/is not Bearer/)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'rejects an active response that carries no token_type (fails closed)' do
+ stub_faraday_response(status: 200, body: active_body.except('token_type'))
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'does not cache a token rejected for its token_type' do
+ conn = stub_faraday_response(status: 200, body: active_body.merge('token_type' => 'DPoP'))
+ described_class.introspect(token)
+ described_class.introspect(token)
+ expect(conn).to have_received(:post).twice
+ end
+
+ it 'rejects a token issued by an unexpected issuer' do
+ stub_faraday_response(status: 200, body: active_body.merge('iss' => 'https://evil.example/oauth'))
+ expect(Rails.logger).to receive(:warn).with(/iss /)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'rejects an active response that carries no iss (fails closed)' do
+ stub_faraday_response(status: 200, body: active_body.except('iss'))
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'derives the expected issuer from the configured introspection URL' do
+ allow(SiteSetting).to receive(:value).with('courses_mooc_fi_introspection_url')
+ .and_return('http://project-331.local/api/v0/main-frontend/oauth/introspect')
+ stub_faraday_response(
+ status: 200,
+ body: active_body.merge('iss' => 'http://project-331.local/api/v0/main-frontend/oauth')
+ )
+ expect(described_class.introspect(token)).not_to be_nil
+ end
+
+ it 'refuses to introspect when the configured URL has no /introspect suffix' do
+ allow(SiteSetting).to receive(:value).with('courses_mooc_fi_introspection_url')
+ .and_return('https://courses.mooc.fi/api/v0/main-frontend/oauth')
+ stub_faraday_response(status: 200, body: active_body)
+ expect(Rails.logger).to receive(:error).with(/does not end in \/introspect/)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'does not cache a token rejected for its issuer' do
+ conn = stub_faraday_response(status: 200, body: active_body.merge('iss' => 'https://evil.example/oauth'))
+ described_class.introspect(token)
+ described_class.introspect(token)
+ expect(conn).to have_received(:post).twice
+ end
+
+ it 'accepts a token whose client is allowed to use bearer tokens' do
+ stub_faraday_response(status: 200, body: active_body.merge('client_bearer_allowed' => true))
+ expect(described_class.introspect(token)).not_to be_nil
+ end
+
+ it 'rejects a token whose client is not allowed to use bearer tokens' do
+ stub_faraday_response(status: 200, body: active_body.merge('client_bearer_allowed' => false))
+ expect(Rails.logger).to receive(:warn).with(/client_bearer_allowed/)
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'rejects an active response that carries no client_bearer_allowed (fails closed)' do
+ stub_faraday_response(status: 200, body: active_body.except('client_bearer_allowed'))
+ expect(described_class.introspect(token)).to be_nil
+ end
+
+ it 'does not cache a token rejected for client_bearer_allowed' do
+ conn = stub_faraday_response(status: 200, body: active_body.merge('client_bearer_allowed' => false))
+ described_class.introspect(token)
+ described_class.introspect(token)
+ expect(conn).to have_received(:post).twice
+ end
+ end
+
+ describe 'caching' do
+ it 'caches positive results and does not re-query the provider' do
+ conn = stub_faraday_response(status: 200, body: active_body)
+
+ first = described_class.introspect(token)
+ second = described_class.introspect(token)
+
+ expect(first.sub).to eq(sub)
+ expect(second.sub).to eq(sub)
+ expect(conn).to have_received(:post).once
+ end
+
+ it 'never caches failures' do
+ conn = stub_faraday_response(status: 500, body: active_body)
+ described_class.introspect(token)
+ described_class.introspect(token)
+ expect(conn).to have_received(:post).twice
+ end
+
+ it 'caps the TTL at MAX_CACHE_TTL for a long-lived token' do
+ allow(cache).to receive(:write).and_call_original
+ stub_faraday_response(status: 200, body: active_body.merge('exp' => (Time.now + 3600).to_i))
+
+ described_class.introspect(token)
+
+ expect(cache).to have_received(:write).with(
+ anything, anything, hash_including(expires_in: described_class::MAX_CACHE_TTL)
+ )
+ end
+
+ it 'uses the remaining lifetime when it is shorter than MAX_CACHE_TTL' do
+ allow(cache).to receive(:write).and_call_original
+ stub_faraday_response(status: 200, body: active_body.merge('exp' => (Time.now + 60).to_i))
+
+ described_class.introspect(token)
+
+ expect(cache).to have_received(:write) do |_key, _value, opts|
+ expect(opts[:expires_in]).to be <= 60
+ expect(opts[:expires_in]).to be > 0
+ end
+ end
+
+ it 'does not cache an already-expired token' do
+ allow(cache).to receive(:write).and_call_original
+ stub_faraday_response(status: 200, body: active_body.merge('exp' => (Time.now - 1).to_i))
+
+ described_class.introspect(token)
+
+ expect(cache).not_to have_received(:write)
+ end
+ end
+end