Diff

--- a/mod_http_oauth2/mod_http_oauth2.lua	Tue Aug 29 23:51:17 2023 +0700
+++ b/mod_http_oauth2/mod_http_oauth2.lua	Tue Feb 06 18:32:01 2024 +0700
@@ -111,7 +111,8 @@
 local registration_options = module:get_option("oauth2_registration_options",
 	{ default_ttl = registration_ttl; accept_expired = not registration_ttl });
 
-local pkce_required = module:get_option_boolean("oauth2_require_code_challenge", false);
+local pkce_required = module:get_option_boolean("oauth2_require_code_challenge", true);
+local respect_prompt = module:get_option_boolean("oauth2_respect_oidc_prompt", false);
 
 local verification_key;
 local sign_client, verify_client;
@@ -138,6 +139,8 @@
 	return client;
 end
 
+local purpose_map = { ["oauth2-refresh"] = "refresh_token"; ["oauth"] = "access_token" };
+
 -- scope : string | array | set
 --
 -- at each step, allow the same or a subset of scopes
@@ -212,12 +215,19 @@
 	return code_expires_in(code) < 0;
 end
 
+-- LRU cache for short-term storage of authorization codes and device codes
 local codes = cache.new(10000, function (_, code)
+	-- If the cache is full and the oldest item hasn't expired yet then we
+	-- might be under some kind of DoS attack, so might as well reject further
+	-- entries for a bit.
 	return code_expired(code)
 end);
 
 -- Clear out unredeemed codes so they don't linger in memory.
 module:daily("Clear expired authorization codes", function()
+	-- The tail should be the least recently touched item, and most likely to
+	-- have expired already, so check and remove that one until encountering
+	-- one that has not expired.
 	local k, code = codes:tail();
 	while code and code_expired(code) do
 		codes:set(k, nil);
@@ -242,7 +252,7 @@
 		type = "modify";
 		condition = "bad-request";
 		code = err_name == "invalid_client" and 401 or 400;
-		text = err_desc and (err_name..": "..err_desc) or err_name;
+		text = err_desc or err_name:gsub("^.", string.upper):gsub("_", " ");
 		extra = { oauth2_response = { error = err_name, error_description = err_desc } };
 	});
 end
@@ -272,7 +282,7 @@
 	local grant = refresh_token_info and refresh_token_info.grant;
 	if not grant then
 		-- No existing grant, create one
-		grant = tokens.create_grant(token_jid, token_jid, default_refresh_ttl, token_data);
+		grant = tokens.create_grant(token_jid, token_jid, nil, token_data);
 	end
 
 	if refresh_token_info then
@@ -284,7 +294,7 @@
 		end
 	end
 	-- in with the new refresh token
-	local refresh_token = refresh_token_info ~= false and tokens.create_token(token_jid, grant.id, nil, nil, "oauth2-refresh");
+	local refresh_token = refresh_token_info ~= false and tokens.create_token(token_jid, grant.id, nil, default_refresh_ttl, "oauth2-refresh");
 
 	if role == "xmpp" then
 		-- Special scope meaning the users default role.
@@ -390,13 +400,15 @@
 	end
 	local granted_scopes, granted_role = filter_scopes(request_username, params.scope);
 
-	if pkce_required and not params.code_challenge then
+	local redirect_uri = get_redirect_uri(client, params.redirect_uri);
+
+	if pkce_required and not params.code_challenge and redirect_uri ~= device_uri and redirect_uri ~= oob_uri then
 		return oauth_error("invalid_request", "PKCE required");
 	end
 
 	local prefix = "authorization_code:";
 	local code = id.medium();
-	if params.redirect_uri == device_uri then
+	if redirect_uri == device_uri then
 		local is_device, device_state = verify_device_token(params.state);
 		if is_device then
 			-- reconstruct the device_code
@@ -419,7 +431,6 @@
 		return oauth_error("temporarily_unavailable");
 	end
 
-	local redirect_uri = get_redirect_uri(client, params.redirect_uri);
 	if redirect_uri == oob_uri then
 		return render_page(templates.oob, { client = client; authorization_code = code }, true);
 	elseif redirect_uri == device_uri then
@@ -630,16 +641,32 @@
 		-- First step: login
 		local username = encodings.stringprep.nodeprep(form.username);
 		local password = encodings.stringprep.saslprep(form.password);
+		-- Many things hooked to authentication-{success,failure} don't expect
+		-- non-XMPP sessions so here's something close enough...
+		local auth_event = {
+			session = {
+				type = "http";
+				ip = request.ip;
+				conn = request.conn;
+				username = username;
+				host = module.host;
+				log = request.log;
+				sasl_handler = { username = username; selected = "x-www-form" };
+				client_id = request.headers.user_agent;
+			};
+		};
 		if not (username and password) or not usermanager.test_password(username, module.host, password) then
+			module:fire_event("authentication-failure", auth_event);
 			return {
 				error = "Invalid username/password";
 			};
 		end
+		module:fire_event("authentication-success", auth_event);
 		return {
 			user = {
 				username = username;
 				host = module.host;
-				token = new_user_token({ username = username; host = module.host; auth_time = os.time() });
+				token = new_user_token({ username = username; host = module.host; amr = { "pwd" } });
 			};
 		};
 	elseif form.user_token and form.consent then
@@ -729,7 +756,7 @@
 -- the redirect_uri is missing or invalid. In those cases, we render an
 -- error directly to the user-agent.
 local function error_response(request, redirect_uri, err)
-	if not redirect_uri or redirect_uri == oob_uri then
+	if not redirect_uri or redirect_uri == oob_uri or redirect_uri == device_uri then
 		return render_error(err);
 	end
 	local q = strict_formdecode(request.url.query);
@@ -738,7 +765,7 @@
 	redirect_uri = redirect_uri
 		.. sep .. http.formencode(err.extra.oauth2_response)
 		.. "&" .. http.formencode({ state = q.state, iss = get_issuer() });
-	module:log("warn", "Sending error response to client via redirect to %s", redirect_uri);
+	module:log("debug", "Sending error response to client via redirect to %s", redirect_uri);
 	return {
 		status_code = 303;
 		headers = {
@@ -751,7 +778,6 @@
 
 local allowed_grant_type_handlers = module:get_option_set("allowed_oauth2_grant_types", {
 	"authorization_code";
-	"password"; -- TODO Disable. The resource owner password credentials grant [RFC6749] MUST NOT be used.
 	"refresh_token";
 	device_uri;
 })
@@ -781,7 +807,7 @@
 	end
 end
 
-local allowed_challenge_methods = module:get_option_set("allowed_oauth2_code_challenge_methods", { "plain"; "S256" })
+local allowed_challenge_methods = module:get_option_set("allowed_oauth2_code_challenge_methods", { "S256" })
 for handler_type in pairs(verifier_transforms) do
 	if not allowed_challenge_methods:contains(handler_type) then
 		module:log("debug", "Challenge method %q disabled", handler_type);
@@ -859,37 +885,56 @@
 	end
 
 	-- The 'prompt' parameter from OpenID Core
-	local prompt = set.new(parse_scopes(params.prompt or "select_account login consent"));
-	if prompt:contains("none") then
-		-- Client wants no interaction, only confirmation of prior login and
-		-- consent, but this is not implemented.
-		return error_response(request, redirect_uri, oauth_error("interaction_required"));
-	elseif not prompt:contains("select_account") then
-		-- TODO If the login page is split into account selection followed by login
-		-- (e.g. password), and then the account selection could be skipped iff the
-		-- 'login_hint' parameter is present.
-		return error_response(request, redirect_uri, oauth_error("account_selection_required"));
-	elseif not prompt:contains("login") then
-		-- Currently no cookies or such are used, so login is required every time.
-		return error_response(request, redirect_uri, oauth_error("login_required"));
-	elseif not prompt:contains("consent") then
-		-- Are there any circumstances when consent would be implied or assumed?
-		return error_response(request, redirect_uri, oauth_error("consent_required"));
-	end
+	local prompt = set.new(parse_scopes(respect_prompt and params.prompt or "select_account login consent"));
 
 	local auth_state = get_auth_state(request);
 	if not auth_state.user then
+		if not prompt:contains("login") then
+			-- Currently no cookies or such are used, so login is required every time.
+			return error_response(request, redirect_uri, oauth_error("login_required"));
+		end
+
 		-- Render login page
 		local extra = {};
 		if params.login_hint then
-			extra.username_hint = (jid.prepped_split(params.login_hint));
+			extra.username_hint = (jid.prepped_split(params.login_hint) or encodings.stringprep.nodeprep(params.login_hint));
+		elseif not prompt:contains("select_account") then
+			-- TODO If the login page is split into account selection followed by login
+			-- (e.g. password), and then the account selection could be skipped iff the
+			-- 'login_hint' parameter is present.
+			return error_response(request, redirect_uri, oauth_error("account_selection_required"));
 		end
 		return render_page(templates.login, { state = auth_state; client = client; extra = extra });
 	elseif auth_state.consent == nil then
-		-- Render consent page
 		local scopes, roles = split_scopes(requested_scopes);
 		roles = user_assumable_roles(auth_state.user.username, roles);
-		return render_page(templates.consent, { state = auth_state; client = client; scopes = scopes+roles }, true);
+
+		if not prompt:contains("consent") then
+			local grants = tokens.get_user_grants(auth_state.user.username);
+			local matching_grant;
+			if grants then
+				for grant_id, grant in pairs(grants) do
+					if grant.data and grant.data.oauth2_client and grant.data.oauth2_client.hash == client.client_hash then
+						if set.new(parse_scopes(grant.data.oauth2_scopes)) == set.new(scopes+roles) then
+							matching_grant = grant_id;
+							break
+						end
+					end
+				end
+			end
+
+			if not matching_grant then
+				return error_response(request, redirect_uri, oauth_error("consent_required"));
+			else
+				-- Consent for these scopes already granted to this exact client, continue
+				auth_state.scopes = scopes + roles;
+				auth_state.consent = "granted";
+			end
+
+		else
+			-- Render consent page
+			return render_page(templates.consent, { state = auth_state; client = client; scopes = scopes+roles }, true);
+		end
 	elseif not auth_state.consent then
 		-- Notify client of rejection
 		if redirect_uri == device_uri then
@@ -923,8 +968,9 @@
 		iss = get_issuer();
 		sub = url.build({ scheme = "xmpp"; path = user_jid });
 		aud = params.client_id;
-		auth_time = auth_state.user.auth_time;
+		auth_time = auth_state.user.iat;
 		nonce = params.nonce;
+		amr = auth_state.user.amr;
 	});
 	local response_type = params.response_type;
 	local response_handler = response_type_handlers[response_type];
@@ -1046,15 +1092,67 @@
 	}
 end
 
+local function handle_introspection_request(event)
+	local request = event.request;
+	local credentials = get_request_credentials(request);
+	if not credentials or credentials.type ~= "basic" then
+		event.response.headers.www_authenticate = string.format("Basic realm=%q", module.host.."/"..module.name);
+		return 401;
+	end
+	-- OAuth "client" credentials
+	if not verify_client_secret(credentials.username, credentials.password) then
+		return 401;
+	end
+
+	local client = check_client(credentials.username);
+	if not client then
+		return 401;
+	end
+
+	local form_data = http.formdecode(request.body or "=");
+	local token = form_data.token;
+	if not token then
+		return 400;
+	end
+
+	local token_info = tokens.get_token_info(form_data.token);
+	if not token_info then
+		return { headers = { content_type = "application/json" }; body = json.encode { active = false } };
+	end
+	local token_client = token_info.grant.data.oauth2_client;
+	if not token_client or token_client.hash ~= client.client_hash then
+		return 403;
+	end
+
+	return {
+		headers = { content_type = "application/json" };
+		body = json.encode {
+			active = true;
+			client_id = credentials.username; -- We don't really know for sure
+			username = jid.node(token_info.jid);
+			scope = token_info.grant.data.oauth2_scopes;
+			token_type = purpose_map[token_info.purpose];
+			exp = token.expires;
+			iat = token.created;
+			sub = url.build({ scheme = "xmpp"; path = token_info.jid });
+			aud = credentials.username;
+			iss = get_issuer();
+			jti = token_info.id;
+		};
+	};
+end
+
+-- RFC 7009 says that the authorization server should validate that only the client that a token was issued to should be able to revoke it. However
+-- this would prevent someone who comes across a leaked token from doing the responsible thing and revoking it, so this is not enforced by default.
 local strict_auth_revoke = module:get_option_boolean("oauth2_require_auth_revoke", false);
 
 local function handle_revocation_request(event)
 	local request, response = event.request, event.response;
 	response.headers.cache_control = "no-store";
 	response.headers.pragma = "no-cache";
-	if request.headers.authorization then
-		local credentials = get_request_credentials(request);
-		if not credentials or credentials.type ~= "basic" then
+	local credentials = get_request_credentials(request);
+	if credentials then
+		if credentials.type ~= "basic" then
 			response.headers.www_authenticate = string.format("Basic realm=%q", module.host.."/"..module.name);
 			return 401;
 		end
@@ -1074,6 +1172,22 @@
 		response.headers.accept = "application/x-www-form-urlencoded";
 		return 415;
 	end
+
+	if credentials then
+		local client = check_client(credentials.username);
+		if not client then
+			return 401;
+		end
+		local token_info = tokens.get_token_info(form_data.token);
+		if not token_info then
+			return 404;
+		end
+		local token_client = token_info.grant.data.oauth2_client;
+		if not token_client or token_client.hash ~= client.client_hash then
+			return 403;
+		end
+	end
+
 	local ok, err = tokens.revoke_token(form_data.token);
 	if not ok then
 		module:log("warn", "Unable to revoke token: %s", tostring(err));
@@ -1084,6 +1198,7 @@
 
 local registration_schema = {
 	title = "OAuth 2.0 Dynamic Client Registration Protocol";
+	description = "This endpoint allows dynamically registering an OAuth 2.0 client.";
 	type = "object";
 	required = {
 		-- These are shown to users in the template
@@ -1098,16 +1213,30 @@
 			type = "array";
 			minItems = 1;
 			uniqueItems = true;
-			items = { title = "Redirect URI"; type = "string"; format = "uri" };
+			items = {
+				title = "Redirect URI";
+				type = "string";
+				format = "uri";
+				examples = {
+					"https://app.example.com/redirect";
+					"http://localhost:8080/redirect";
+					"com.example.app:/redirect";
+					oob_uri;
+					device_uri;
+				};
+			};
 		};
 		token_endpoint_auth_method = {
 			title = "Token Endpoint Authentication Method";
+			description = "Authentication method the client intends to use. Recommended is `client_secret_basic`. \z
+			`none` is only allowed for use with the insecure Implicit flow.";
 			type = "string";
 			enum = { "none"; "client_secret_post"; "client_secret_basic" };
 			default = "client_secret_basic";
 		};
 		grant_types = {
 			title = "Grant Types";
+			description = "List of grant types the client intends to use.";
 			type = "array";
 			minItems = 1;
 			uniqueItems = true;
@@ -1129,8 +1258,9 @@
 		application_type = {
 			title = "Application Type";
 			description = "Determines which kinds of redirect URIs the client may register. \z
-			The value 'web' limits the client to https:// URLs with the same hostname as in 'client_uri' \z
-			while the value 'native' allows either loopback http:// URLs or application specific URIs.";
+			The value `web` limits the client to `https://` URLs with the same hostname as \z
+			in `client_uri` while the value `native` allows either loopback URLs like \z
+			`http://localhost:8080/` or application specific URIs like `com.example.app:/redirect`.";
 			type = "string";
 			enum = { "native"; "web" };
 			default = "web";
@@ -1150,10 +1280,12 @@
 		};
 		client_uri = {
 			title = "Client URL";
-			description = "Should be an link to a page with information about the client.";
+			description = "Should be an link to a page with information about the client. \z
+			The hostname in this URL must be the same as in every other '_uri' property.";
 			type = "string";
 			format = "uri";
 			pattern = "^https:";
+			examples = { "https://app.example.com/" };
 		};
 		logo_uri = {
 			title = "Logo URL";
@@ -1161,11 +1293,13 @@
 			type = "string";
 			format = "uri";
 			pattern = "^https:";
+			examples = { "https://app.example.com/appicon.png" };
 		};
 		scope = {
 			title = "Scopes";
 			description = "Space-separated list of scopes the client promises to restrict itself to.";
 			type = "string";
+			examples = { "openid xmpp" };
 		};
 		contacts = {
 			title = "Contact Addresses";
@@ -1177,17 +1311,19 @@
 		tos_uri = {
 			title = "Terms of Service URL";
 			description = "Link to Terms of Service for the client, presented to the user in the consent dialog. \z
-			MUST be a https:// URL with hostname matching that of 'client_uri'.";
+			MUST be a `https://` URL with hostname matching that of `client_uri`.";
 			type = "string";
 			format = "uri";
 			pattern = "^https:";
+			examples = { "https://app.example.com/tos.html" };
 		};
 		policy_uri = {
 			title = "Privacy Policy URL";
-			description = "Link to a Privacy Policy for the client. MUST be a https:// URL with hostname matching that of 'client_uri'.";
+			description = "Link to a Privacy Policy for the client. MUST be a `https://` URL with hostname matching that of `client_uri`.";
 			type = "string";
 			format = "uri";
 			pattern = "^https:";
+			examples = { "https://app.example.com/policy.pdf" };
 		};
 		software_id = {
 			title = "Software ID";
@@ -1200,7 +1336,7 @@
 			description = "Version of the client software being registered. \z
 			E.g. to allow revoking all related tokens in the event of a security incident.";
 			type = "string";
-			example = "2.3.1";
+			examples = { "2.3.1" };
 		};
 	};
 }
@@ -1221,6 +1357,9 @@
 
 local function redirect_uri_allowed(redirect_uri, client_uri, app_type)
 	local uri = url.parse(redirect_uri);
+	if not uri then
+		return false;
+	end
 	if not uri.scheme then
 		return false; -- no relative URLs
 	end
@@ -1232,8 +1371,24 @@
 end
 
 function create_client(client_metadata)
-	if not schema.validate(registration_schema, client_metadata) then
-		return nil, oauth_error("invalid_request", "Failed schema validation.");
+	local valid, validation_errors = schema.validate(registration_schema, client_metadata);
+	if not valid then
+		return nil, errors.new({
+			type = "modify";
+			condition = "bad-request";
+			code = 400;
+			text = "Failed schema validation.";
+			extra = {
+				oauth2_response = {
+					error = "invalid_request";
+					error_description = "Client registration data failed schema validation."; -- TODO Generate from validation_errors?
+					-- JSON Schema Output Format
+					-- https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-01#name-basic
+					valid = false;
+					errors = validation_errors;
+				};
+			};
+		});
 	end
 
 	local client_uri = url.parse(client_metadata.client_uri);
@@ -1289,6 +1444,15 @@
 		return nil, oauth_error("invalid_client_metadata", "No allowed 'response_types' specified");
 	end
 
+	if client_metadata.token_endpoint_auth_method ~= "none" then
+		-- Ensure that each client_id JWT with a client_secret is unique.
+		-- A short ID along with the issued at timestamp should be sufficient to
+		-- rule out brute force attacks.
+		-- Not needed for public clients without a secret, but those are expected
+		-- to be uncommon since they can only do the insecure implicit flow.
+		client_metadata.nonce = id.short();
+	end
+
 	-- Do we want to keep everything?
 	local client_id = sign_client(client_metadata);
 
@@ -1296,14 +1460,7 @@
 	client_metadata.client_id_issued_at = os.time();
 
 	if client_metadata.token_endpoint_auth_method ~= "none" then
-		-- Ensure that each client_id JWT with a client_secret is unique.
-		-- A short ID along with the issued at timestamp should be sufficient to
-		-- rule out brute force attacks.
-		-- Not needed for public clients without a secret, but those are expected
-		-- to be uncommon since they can only do the insecure implicit flow.
-		client_metadata.nonce = id.short();
-
-		local client_secret = make_client_secret(client_id, client_metadata);
+		local client_secret = make_client_secret(client_id);
 		client_metadata.client_secret = client_secret;
 		client_metadata.client_secret_expires_at = 0;
 
@@ -1424,6 +1581,9 @@
 		-- Step 5. Revoke token (access or refresh)
 		["POST /revoke"] = handle_revocation_request;
 
+		-- Get info about a token
+		["POST /introspect"] = handle_introspection_request;
+
 		-- OpenID
 		["GET /userinfo"] = handle_userinfo_request;
 
@@ -1432,7 +1592,7 @@
 			headers = {
 				["Content-Type"] = "text/css";
 			};
-			body = render_html(templates.css, module:get_option("oauth2_template_style"));
+			body = templates.css;
 		} or nil;
 		["GET /script.js"] = templates.js and {
 			headers = {
@@ -1445,6 +1605,7 @@
 		["GET /register"] = { headers = { content_type = "application/schema+json" }; body = json.encode(registration_schema) };
 		["GET /token"] = function() return 405; end;
 		["GET /revoke"] = function() return 405; end;
+		["GET /introspect"] = function() return 405; end;
 	};
 });
 
@@ -1481,6 +1642,8 @@
 		revocation_endpoint = handle_revocation_request and module:http_url() .. "/revoke" or nil;
 		revocation_endpoint_auth_methods_supported = array({ "client_secret_basic" });
 		device_authorization_endpoint = handle_device_authorization_request and module:http_url() .. "/device";
+		introspection_endpoint = handle_introspection_request and module:http_url() .. "/introspect";
+		introspection_endpoint_auth_methods_supported = nil;
 		code_challenge_methods_supported = array(it.keys(verifier_transforms));
 		grant_types_supported = array(it.keys(grant_type_handlers));
 		response_modes_supported = array(it.keys(response_type_handlers)):map(tmap { token = "fragment"; code = "query" });