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

Already on GitHub? Sign in to your account

Add an openidish mechanism for proving that you own a given user_id #765

Merged
merged 2 commits into from May 5, 2016

Conversation

Projects
None yet
2 participants
Contributor

NegativeMjark commented May 5, 2016

Doesn't actually fully implement open id yet. but the intention is that response format could be made compatible with that expected from open id.

However it does have enough machinery to prove ownership of a given user_id which is probably a good start.

@erikjohnston erikjohnston and 1 other commented on an outdated diff May 5, 2016

synapse/rest/client/v2_alpha/openid.py
+class IdTokenServlet(RestServlet):
+ """
+ Get a bearer token that may be passed to a third party to confirm ownership
+ of a matrix user id.
+
+ The format of the response could be made compatible with the format given
+ in http://openid.net/specs/openid-connect-core-1_0.html#TokenResponse
+
+ But instead of returning a signed "id_token" the response contains the
+ name of the issuing matrix homeserver. This means that for now the third
+ party will need to check the validity of the "id_token" against the
+ federation /openid/userinfo endpoint of the homeserver.
+
+ Request:
+
+ POST /user/{user_id}/openid/token?access_token=... HTTP/1.1
@erikjohnston

erikjohnston May 5, 2016

Owner

This doesn't feel obvious that this is provisioning a new token, rather than something else.

Maybe GET or /openid/provision_token?

@erikjohnston

erikjohnston May 5, 2016

Owner

Eww. Would still prefer at least /user/{user_id}/openid/request/token or something, anything really

@erikjohnston erikjohnston commented on the diff May 5, 2016

synapse/federation/transport/server.py
@@ -448,6 +448,50 @@ def _wrap(self, code):
return code
+class OpenIdUserInfo(BaseFederationServlet):
+ """
+ Exchange a bearer token for information about a user.
+
+ The response format should be compatible with:
+ http://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse
+
+ GET /openid/userinfo?access_token=ABDEFGH HTTP/1.1
@erikjohnston

erikjohnston May 5, 2016

Owner

Is access_token something part of the OpenID spec? Or defined by us? I'd rather not have two different meanings for access_token

@NegativeMjark

NegativeMjark May 5, 2016

Contributor

It's part of the OpenID/Oauth spec I'm afraid.

@erikjohnston erikjohnston commented on the diff May 5, 2016

synapse/rest/client/v2_alpha/openid.py
+ self.auth = hs.get_auth()
+ self.store = hs.get_datastore()
+ self.clock = hs.get_clock()
+ self.server_name = hs.config.server_name
+
+ @defer.inlineCallbacks
+ def on_POST(self, request, user_id):
+ requester = yield self.auth.get_user_by_req(request)
+ if user_id != requester.user.to_string():
+ raise AuthError(403, "Cannot request tokens for other users.")
+
+ # Parse the request body to make sure it's JSON, but ignore the contents
+ # for now.
+ parse_json_object_from_request(request)
+
+ token = random_string(24)
@erikjohnston

erikjohnston May 5, 2016

Owner

Should this be a macaroon to keep it consistent with the other tokens?

@NegativeMjark

NegativeMjark May 5, 2016

Contributor

Maybe, something we can change down the line if needed.

Owner

erikjohnston commented May 5, 2016

LGTM

@NegativeMjark NegativeMjark merged commit 1f590f3 into develop May 5, 2016

8 checks passed

Flake8 + Packaging (Commit) Build #603 origin/markjh/open_id succeeded in 33 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #587 origin/markjh/open_id succeeded in 4 min 40 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #594 origin/markjh/open_id succeeded in 3 min 21 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #647 origin/markjh/open_id succeeded in 1 min 16 sec
Details
Unit Tests (Merged PR) Build finished.
Details

@richvdh richvdh deleted the markjh/open_id branch Dec 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment