Dexと組み合わせたMediawikiが認証後のリダイレクトでエラーになる現象について

# はじめに

MediawikiでOIDCを利用する際に、サイトの言語設定を日本語にすると、リダイレクト先のページが"/Special:PluggableAuth"から"/特別:PluggableAuth"になます。

この日本語を含むURLをRedirectURIに指定すると、パスワード認証後の最終的な検証処理時にエラーとなります。

これはMediawikiに限らず日本語などの8bit文字以外を含むURLが%表記によってエスケープされた場合に、OIDC IDプロバイダーであるDex側が適切にURLをデコードせずに検証を行っていることが原因です。

```c:
// 問題となっているredirect_uriの検証プロセスを示す疑似コード
if ("/特別:PluggableAuth" == "/%E7%89%B9%E5%88%A5:PluggableAuthLogin") {
  // %表記の部分がデコードされておらず、正常時の処理に遷移しない
}
```

この経緯と、開発側に提案した改善案についてまとめておきます。

# 参考資料

* [Mediawiki - Manual:デバッグの方法](https://www.mediawiki.org/wiki/Manual:How_to_debug/ja)

## Mediawikiをデバッグするための変更

MediawikiのLocalSettings.phpには、次のようにデバッグの出力先を含めてコードを追加する必要があります。

```php:LocalSettings.phpにデバッグ用のコードを指定する例
error_reporting( -1 );
ini_set( 'display_errors', 1 );

$wgShowDebug = true;
```

参考資料をぱっとみて最初の2行だけで十分だと思ったのですが、画面に何も表示されなくて焦りました。

# 分析結果

Mediawiki側のデバッグコンソールでは次のように出力されています。

```text:デバッグコンソール上の抜粋
[PluggableAuth] Plugin name: OpenIDConnect
[OpenIDConnect] Redirect URL: http://mw139.example.org/mediawiki/index.php/%E7%89%B9%E5%88%A5:PluggableAuthLogin
[OpenIDConnect] Jumbojett\OpenIDConnectClientException: redirect_uri did not match URI from initial request. in /var/www/html/mediawiki/extensions/OpenIDConnect/vendor/jumbojett/openid-connect-php/src/OpenIDConnectClient.php:310
```

最初にDexからリダイレクトした際に検証は通っていて、最後のリダイレクトでエラーになっているのでDex側の対応が不十分な可能性が高そうです。

```php:OpenIDConnect.phpの該当コード
// OIDCサーバーに接続する際に、RedirectURLを取得するコード
$redirectURL = SpecialPage::getTitleFor( 'PluggableAuthLogin' )->getFullURL();
$oidc->setRedirectURL( $redirectURL );
$this->logger->debug( 'Redirect URL: ' . $redirectURL );
```

デバッグログの該当部分を確認します。

```php:OpenIDConnectClient.phpの310行前後
$code = $_REQUEST['code'];
$token_json = $this->requestTokens($code);

// Throw an error if the server returns one
if (isset($token_json->error)) {
    if (isset($token_json->error_description)) {
        throw new OpenIDConnectClientException($token_json->error_description);
    }
    throw new OpenIDConnectClientException('Got response: ' . $token_json->error);
}
```

``requestTokens()``の内部で何をやっているか確認していって、最終的にPHP JSONライブラリの``json_decode()``で[token_endpoint](https://dex.example.org/dex/token)に対して検証リクエストを出していて、その時の引数が次のようになっています。

```
redirect_uri=http%3A%2F%2Fmw139.example.org%2Fmediawiki%2Findex.php%2F%25E7%2589%25B9%25E5%2588%25A5%3APluggableAuthLogin
```

"特別:PluggableAuthLogin"の部分がデコードされているところに、さらにデコードされていて、よく分からない感じになっていてます。これでも問題なさそうですが、ここから先はPHPのJSONモジュールの仕事になってきますが、共有ライブラリになっているので簡単には手が出せない感じです。

一度、wgLanguageCodeを"en"にしてから、どんな処理になっているか確認しておきます。


```
redirect_uri=http%3A%2F%2Fmw139.example.org%2Fmediawiki%2Findex.php%2FSpecial%3APluggableAuthLogin
```

このリクエストをtoken_endpointが受け取った結果、何等かのエラーが発生しているので、ここの処理を確認します。

dexサーバー側でデバッグメッセージを出力するように改造すると、次のようなミスマッチが発生していることが分かりました。

```text:
msg="authCode.RedirectURI http://mw139.example.org/mediawiki/index.php/特別:PluggableAuthLogin"
msg="redirectURI http://mw139.example.org/mediawiki/index.php/%E7%89%B9%E5%88%A5:PluggableAuthLogin"
```

[https://github.com/oidc-wp/openid-connect-generic/pull/289/files](https://github.com/oidc-wp/openid-connect-generic/pull/289/files) では、redirect_uriの値に対して、rawurlencode() が適用されています。

規約的には、POSTする際の値は%エスケープされているべきだという以外のルールはなさそうです。

## Dex IdP側の実装を確認

Dexサーバー側の処理をみると、server/handlers.goの、handleAuthCode()の中でredirectURIを比較していることが分かります。
この部分の処理は次のようになっています。

```go:server/handlers.goのhandleAuthCode()から抜粋
	if authCode.RedirectURI != redirectURI {
		s.tokenErrHelper(w, errInvalidRequest, "redirect_uri did not match URI from initial request.", http.StatusBadRequest)
		return
	}
```

最初のエラーメッセージを出力しているのは、ここの戻り値をMediawiki側で出力していたことが分かります。

if文条件の前者のauthCode.RedirectURIは、``url.QueryUnescape(q.Get("redirect_uri"))`` の戻り値が保存されています。

後者はのredirectURIは、``r.PostFormValue("redirect_uri")`` の戻り値となっていて、比較する際にはurl.QueryUnescape()されていない文字列と比較しているので不整合が発生します。

OpenID ConnectのSpecificationを読む限りは、サーバーに渡される値がurlencode()されていることは当然の処理だと思うので、サーバー側で受け取った値を比較する際に、url.QueryUnescape()で処理していないことは問題のように感じられます。

Specificationを読んでも、redirect_uriは検証しなければならないとは書かれていますが、どのようにエンコードされているかについては直接は言及されていません。

次のようなパッチを作成して、動作することまでは確認しました。

```patch:dexidp/dexのrefs/tags/v2.35.3に対する差分
diff --git a/server/handlers.go b/server/handlers.go
index 11dcdd07..be50fdf5 100755
--- a/server/handlers.go
+++ b/server/handlers.go
@@ -832,7 +832,11 @@ func (s *Server) calculateCodeChallenge(codeVerifier, codeChallengeMethod string
 // handle an access token request https://tools.ietf.org/html/rfc6749#section-4.1.3
 func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client storage.Client) {
        code := r.PostFormValue("code")
-       redirectURI := r.PostFormValue("redirect_uri")
+       redirectURI, err := url.QueryUnescape(r.PostFormValue("redirect_uri"))
+       if err != nil {
+               s.tokenErrHelper(w, errInvalidRequest, "No redirect_uri provided.", http.StatusBadRequest)
+               return
+       }
 
        if code == "" {
                s.tokenErrHelper(w, errInvalidRequest, `Required param: code.`, http.StatusBadRequest)
```

この顛末をまとめて、issuesとしてDex側に報告しています。

* [https://github.com/dexidp/dex/issues/2766](https://github.com/dexidp/dex/issues/2766)

さらにPull Requestを提出しました。

* [https://github.com/dexidp/dex/pull/3492](https://github.com/dexidp/dex/pull/3492)

対応はここまでですが、本来どのように処理されるべきなのか調べてみます。

## PRを出し直しました

元のPRはhandlers.goの中でurl.QueryUnescape()を追加していましたがクライアントから渡されたパラメータを変換するのは影響範囲が広そうなので、oauth2.goを修正して最初のサーバーからのredirect_uriのパラメータをQueryUnescape()でチェックしつつ未処理のURLを保存するように改めました。

* [https://github.com/dexidp/dex/pull/3733](https://github.com/dexidp/dex/pull/3733)

```diff:
diff --git a/server/oauth2.go b/server/oauth2.go
index ec972bea..0fa7d4bc 100644
--- a/server/oauth2.go
+++ b/server/oauth2.go
@@ -457,7 +457,8 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques
                return nil, newDisplayedErr(http.StatusBadRequest, "Failed to parse request.")
        }
        q := r.Form
-       redirectURI, err := url.QueryUnescape(q.Get("redirect_uri"))
+       redirectURI := q.Get("redirect_uri")
+       _, err := url.QueryUnescape(redirectURI)
        if err != nil {
                return nil, newDisplayedErr(http.StatusBadRequest, "No redirect_uri provided.")
        }
```

その代わり、サーバー側の設定ファイルはエスケープしたURLで指定することになりますが、この挙動自体はKeyCloakと同じになるだけです。

```text:Dexサーバー側の設定ファイル
redirectURIs:
  - http://localhost:8000/mediawiki/index.php/%E7%89%B9%E5%88%A5:PluggableAuthLogin
```

みかけは正規化されたようですし、ユーザーは設定する時にちょっと面倒ですが前の変更よりは良くなったと思います。

# Keycloakの処理と比較してみた

URLにおける``.../特別:PluggableAuth``のような指定をどのようにすればいいのか、KeyCloakのDockerコンテナをstart-devモードで起動して確認しました。

## 環境

* KeyCloak - quay.io/keycloak/keycloak:24.0.3
* MediaWiki - v1.39.7
* OpenID Connect Extension - OpenIDConnect: REL1_39 2024-04-17T01:17:23 f193bef v8.0.3
* PluggableAuth Extension - PluggableAuth: REL1_39 2024-03-04T07:17:33 1884a12 v7.1.0

## non-ascii文字列を含む redirect_uri の挙動

KeyCloakでは正常に稼動するredirect_uriの指定は次のようになります。

```text:
* https://example.com/mediawiki/index.php/%E7%89%B9%E5%88%A5:PluggableAuthLogin
* https://example.com/mediawiki/index.php/Special:PluggableAuthLogin
```

Dexとの違いは比較の際にKeyCloakは%エスケープされた文字列のまま比較します。Valid redirect URIsのエントリにはそのままエスケープされた状態の文字列を指定しています。

人間には少し対応が難しいかもしれませんが、この状態でKeyCloakは正常に動作します。

## Dex側の挙動の確認

無修正のDex(v2.39.1)に対して同様の設定でアクセスしようとすると次のようなエラーが表示されます。

![image.png](https://qiita-image-store.s3.ap-northeast-1.amazonaws.com/0/78296/0f880a6e-df25-4e68-8db1-55bd7451be7d.png)

Dex側のログには次のように記録されています。

```text:
time="2024-04-21T14:06:38Z" level=error msg="Failed to parse authorization request: Unregistered redirect_uri (\"http://mw139.x200.yadiary.net/mediawiki/index.php/特別:PluggableAuthLogin\")."
```

Dex側はQuery文字列をunescapeした状態で扱うため、内部ではUTF-8文字列として``redirectURIs``のリストと比較しています。

このためDexでは``redirectURIs``にunescapeされた状態の文字列(UTF-8)を入力する必要があります。これは人間が認知しやすいので良いとは思うのですが、問題はDex側でredirect_uriを比較する際に十分にunescape処理を徹底していないために問題が発生します。

## URIの比較はどのように行われるべきか

OpenID Connect 1.0の仕様書では次のように記載されています。

```text:openID Connect 1.0 - Coreからの抜粋
3.2.2.1.  Authentication Request
...
redirect_uri    REQUIRED. Redirection URI to which the response will be sent. 
This URI MUST exactly match one of the Redirection URI values for the Client 
pre-registered at the OpenID Provider, with the matching performed as described in
Section 6.2.1 of [RFC3986] (Simple String Comparison). When using this flow, 
the Redirection URI MUST NOT use the http scheme unless the Client is a native 
application, in which case it MAY use the http scheme with localhost or the IP 
loopback literals 127.0.0.1 or [::1] as the hostname.
...
```

loopbackアクセスでなければ常にHTTPSを使いなさいという事とRFC3986のSimple String Comparisonに従いなさいというのが仕様書の指示になっています。

RFC3986の該当セクションの記載は、それほど明快ではありませんが、normalizeは不要なものの、ある程度のconversionは妥当だと認められているようです。

```text:RFC3986 Simple String Comparisonからの抜粋
In practical terms, character-by-character comparisons should be done 
codepoint-by-codepoint after conversion to a common character encoding.
```

url.QueryUnescape()の処理がconversionの範疇かは微妙ですが、common character encodingに統一すればその有無は関係ないように読めます。

ユーザーが事前にOIDC Providerへ登録した文字列と``MUST exactly match`` という仕様の要求を満す方法として、KeyCloakでは積極的な処理はしない、Dexのように表面的なデコード処理を行うといった処理を行う、のはどちらも妥当に思えます。

OIDCの利用者としてはredirect_uriにNon-ASCII文字列を指定することは極力避けた方が良いでしょう。

以上