Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URL-encoded parameters are dropped from the state parameter during login #773

Closed
abias opened this issue Nov 23, 2024 · 2 comments
Closed

Comments

@abias
Copy link

abias commented Nov 23, 2024

Thank you very much for this helpful project!
I would like to use it to mock a OAuth2 provider to automatically test a OAuth2 plugin in Moodle LMS.

However, I had to see that logins with the mocked OAuth2 provider always fail, obviously due to the fact that URL-encoded parameters are dropped from the state parameter when redirecting back to the Moodle application.

This is what happens in my test setup:


  1. The application redirects the user to the OAuth2 server on 8080 with this URL:

https://localhost:8080/default/authorize?client_id=foo&response_type=code&redirect_uri=http%3A%2F%2Flocalhost%3A61504%2Fadmin%2Foauth2callback.php&state=%2Fauth%2Foauth2%2Flogin.php%3Fwantsurl%3Dhttp%253A%252F%252Flocalhost%253A61504%252F%26sesskey%3DRQ9OUF5ATR%26id%3D2&scope=openid%20profile%20email

Please note that the state parameter contains a URL with URL-encoded parameters wantsurl, sesskey and id.
This state should make it back to the application as-is when the OAuth2 server redirects the user back to the application.


  1. On the OAuth2 server login page, I see this at the bottom of the page:
Authorization Request
----------------------
client_id = foo
response_type = code
redirect_uri = http://localhost:61504/admin/oauth2callback.php
state = /auth/oauth2/login.php?wantsurl
sesskey = RQ9OUF5ATR
id = 2
scope = openid profile email

This is already wrong, the OAuth2 server seems to have split up the URL-encoded parameters.


  1. Then I log in by entering this data into the OAuth2 server login form:

User: john
Claims:

{
    "lastname": "Doe",
    "firstname": "John",
    "email": "[email protected]",
    "username": "john",
    "sub": "john",
    "updated_at": 1490198843,
}

  1. The login form sends a POST request to the OAuth2 server to this URL:

https://localhost:8080/default/authorize?client_id=foo&response_type=code&redirect_uri=http://localhost:61504/admin/oauth2callback.php&state=/auth/oauth2/login.php?wantsurl=http%3A%2F%2Flocalhost%3A61504%2F&sesskey=RQ9OUF5ATR&id=2&scope=openid profile email

And the OAuth2 server redirects the user to this URL:

http://localhost:61504/admin/oauth2callback.php?code=e8Bx2HKPMfGh4-zOr4xGOEIS_gzdv5Qjnh07wKduiXE&state=%2Fauth%2Foauth2%2Flogin.php%3Fwantsurl%3Dhttp%253A%252F%252Flocalhost%253A61504%252F

You can see the request and the answer in this screenshot here as well:

grafik

You can see that, in the POST URL, the sesskey and id parameters are added with a real & instead of a URL-encoded %26 to the URL.
And because of that, they are not part of the state parameter anymore and are missing when the OAuth2 server adds the state parameter to the redirect URL.

Unfortunately, as my application needs this sesskey parameter, the login fails.


I would highly appreciate if you could check if this behaviour is a bug in the OAuth2 server which can be fixed.
Or if I am doing anything wrong when using the OAuth2 server.

@tommytroen
Copy link
Collaborator

Hi @abias! Thanks for filing this issue!

I had a quick look, and it seems that the sesskey and id parameters gets interpreted as separate (custom) params by the AuthenticationRequest class from the library we are using - Nimbus. At first glance this doesn't seem fixable from our side without removing the Nimbus library for this flow.

A more robust approach you could do from your side is to for instance base64 encode (or some other encoding that doesnt conflict with the url encoding) the entire state value (i.e. wantsurl, sesskey and id) and send that to the oauth server, then decode it when receiving the redirect. That should work with both this OAuth server and any other production server. Let me know how it goes 😄

@abias
Copy link
Author

abias commented Dec 11, 2024

Hi @tommytroen ,

thank you very much for your analysis and feedback.

I fully understand that you cannot fix this issue as you rely on a third party library.
Rewriting the Auth request on my side is not possible unfortunately as I am testing a third party application as well.
Instead / in the meantime, I was able to rewrite my tests with the help of Wiremock.

Here are the three Wiremock mappings which I used for anyone who is interested:

diff --git a/tests/fixtures/wiremock-mappings/authorize.json b/tests/fixtures/wiremock-mappings/authorize.json
new file mode 100644
index 0000000..4022b24
--- /dev/null
+++ b/tests/fixtures/wiremock-mappings/authorize.json
@@ -0,0 +1,13 @@
+{
+    "request": {
+      "method": "GET",
+      "urlPath": "/oauth/authorize"
+    },
+    "response": {
+      "status": 302,
+      "headers": {
+        "Location": "{{request.query.redirect_uri}}?state={{urlEncode request.query.state}}&code=foo"
+      },
+      "transformers": ["response-template"]
+    }
+  }
\ No newline at end of file
diff --git a/tests/fixtures/wiremock-mappings/token.json b/tests/fixtures/wiremock-mappings/token.json
new file mode 100644
index 0000000..8cd5350
--- /dev/null
+++ b/tests/fixtures/wiremock-mappings/token.json
@@ -0,0 +1,15 @@
+{
+    "request": {
+      "method": "POST",
+      "urlPath": "/oauth/token"
+    },
+    "response": {
+      "status": 200,
+      "jsonBody": {
+        "access_token": "mock_access_token",
+        "token_type": "Bearer",
+        "expires_in": 3600,
+        "refresh_token": "mock_refresh_token"
+      }
+    }
+  }
\ No newline at end of file
diff --git a/tests/fixtures/wiremock-mappings/userinfo.json b/tests/fixtures/wiremock-mappings/userinfo.json
new file mode 100644
index 0000000..c6f9be3
--- /dev/null
+++ b/tests/fixtures/wiremock-mappings/userinfo.json
@@ -0,0 +1,20 @@
+{
+    "request": {
+      "method": "GET",
+      "urlPath": "/oauth/userinfo"
+    },
+    "response": {
+      "status": 200,
+      "headers": {
+        "Content-Type": "application/json"
+      },
+      "jsonBody": {
+        "sub": "1234567890",
+        "name": "John Doe",
+        "given_name": "John",
+        "family_name": "Doe",
+        "preferred_username": "johndoe",
+        "email": "[email protected]"
+      }
+    }
+  }
\ No newline at end of file

From my point of view, you can close this issue.

Cheers,
Alex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants