andstatus / andstatus

Multiple accounts client for multiple Social networks. For Android
http://andstatus.org/
Apache License 2.0
307 stars 69 forks source link

Support /.well-known/oauth-authorization-server for ActivityPub #561

Closed JD557 closed 1 year ago

JD557 commented 1 year ago

As per the OAuth spec - Section 3: https://www.rfc-editor.org/rfc/rfc8414#section-3

Authorization servers supporting metadata MUST make a JSON document containing metadata as specified in Section 2 available at a path formed by inserting a well-known URI string into the authorization server's issuer identifier between the host component and the path component, if any. By default, the well-known URI string used is "/.well-known/oauth-authorization-server".

It would be nice to use the endpoints specified here if possible (falling back to the defaults if this endpoint is unreachable or if the file is invalid).

Related to #499

Hopefully, this would simplify the integration with some applications such as microblog.pub (issue regarding implementing the client protocol: https://todo.sr.ht/~tsileo/microblog.pub/48).

yvolk commented 1 year ago

Hi @JD557, thanks for the suggestion. Could you arrange test endpoint / account for me to understand details and to test that changes work. Please email connection details to me.

yvolk commented 1 year ago

Hello @JD557. I see that currently microblog.pub metadata doesn't have this (defined in https://www.rfc-editor.org/rfc/rfc8414#section-2): " registration_endpoint OPTIONAL. URL of the authorization server's OAuth 2.0 Dynamic Client Registration endpoint [RFC7591]. " AndStatus starts with client registration, and it fails because of absence of this endpoint.

I tried different "known to me" endpoints but they all return 404: "api/v1/apps" - like Mastodon (i.e. post to https://test-blog.joaocosta.eu/api/v1/apps ) "api/client/register" - like pump.io "client/register" "register"

Does microblog.pub support Dynamic Client registration?

Similar discussion on Client Registration endpoint is around here: https://github.com/andstatus/andstatus/issues/548#issuecomment-1014756554

JD557 commented 1 year ago

Doesn't look like it.

Also, looking at the IndieAuth spec (which is the extension implemented):

Clients are identified by a [URL]. Client identifier URLs MUST have either an https or http scheme, MUST contain a path component, MUST NOT contain single-dot or double-dot path segments, MAY contain a query string component, MUST NOT contain a fragment component, MUST NOT contain a username or password component, and MAY contain a port. Additionally, hostnames MUST be domain names or a loopback interface and MUST NOT be IPv4 or IPv6 addresses except for IPv4 127.0.0.1 or IPv6 [::1].

Client registration at the authorization endpoint is not necessary, since client IDs are resolvable URLs

Although I admit I'm don't know if an app should just use 127.0.0.1 or something else.

Also, I'm not sure if there's a clean way to find out "this server requires registration" or "just use an URL"... Maybe that can be inferred from the registration_endpoint (if it's undefined, just use an URL as a client ID)?

yvolk commented 1 year ago

I think that implementation of a Dynamic client registration is a good thing. Below are my attempts showing that more changes are needed...

For the test I patched code to succeed with Dynamic Client Registration and assigned clientKey (id?!): "http://andstatus.org" and secret: "fake_consumer_secret" login screen with password prompt opened, but after typing password I was redirected to https://test-blog.joaocosta.eu/auth?scope=read%20write%20follow and saw "Internal Server Error" on that page. Obviously the server didn't know, where to get "redirect_uris" of AndStatus client app that normally are taken during Client registration...

Looks like I need to point client_id to a page with some information specified by https://www.w3.org/TR/indieauth/#client-information-discovery...

JD557 commented 1 year ago

OK, then I guess that OAuth implementation is not as complete as I thought :(

Unfortunately, I don't know any other example that uses /.well-known/oauth-authorization-server. Sorry if I wasted your time.

yvolk commented 1 year ago

I think that I will implement support of endpoints defined in the /.well-known/oauth-authorization-server, as you see I mentioned this already in https://github.com/andstatus/andstatus/issues/548#issuecomment-1017288931 So thank you for reminding me that this feature is really needed in order to get rid of Mastodon's hardcoded endpoints.

But looks like this won't work for you without implementation of Dynamic Client Registration in microblog.pub

yvolk commented 1 year ago

Hi @JD557 I added Support of "OAuth 2.0 Authorization Server Metadata" (RFC 8414) see v.59.11 Beta, see its build at #456 In order for us to get a feeling that this actually works, at least partially, I included a hack into a code: If Dynamic Client Registration fails AND host is "test-blog.joaocosta.eu" then fake client key (id) and secret are used, and the process continues (using the Authorization Server Metadata from "/.well-known/oauth-authorization-server").

Please check!

JD557 commented 1 year ago

Thanks There was indeed a bug in microblog.pub which was now fixed.

Running it now on the app I can get the login flow, but somehow the redirect to http://oauth-redirect.andstatus.org/ doesn't seem to do anything (I checked on the browser and it sends the code, state and iss parameters). Not sure if that was expected.

yvolk commented 1 year ago

The redirect doesn't work because of the bug on the web page, see below.

  1. There is still no proper Client Registration. This is when AndStatus sends this info:
    {
    "client_name": "AndStatus",
    "redirect_uris": "http:\/\/oauth-redirect.andstatus.org",
    "scopes": "read write follow",
    "website": "http:\/\/andstatus.org",
    }

As the "registration_endpoint" is absent in the Authorization Server Metadata, AndStatus sends this registration request to (from log)

registerClient; for https://test-blog.joaocosta.eu; URL='https://test-blog.joaocosta.eu/api/v1/apps'

gets error and applies

Fake Client registration
  1. Then we see:

    Retrieving request token for test-user@test-blog.joaocosta.eu/ActivityPub
    ...
    Starting Web view at https://test-blog.joaocosta.eu/auth?scope=read%20write%20follow&response_type=code&client_id=http%3A%2F%2Fandstatus.org&redirect_uri=http%3A%2F%2Foauth-redirect.andstatus.org&state=state_1436_1671163704135
    ...
    onPageStarted: https://test-blog.joaocosta.eu/admin/login?redirect=https%3A//test-blog.joaocosta.eu/auth%3Fscope%3Dread%2520write%2520follow%26response_type%3Dcode%26client_id%3Dhttp%253A%252F%252Fandstatus.org%26redirect_uri%3Dhttp%253A%252F%252Foauth-redirect.andstatus.org%26state%3Dstate_1436_1671163704135

    I type correct password on that page, press "login" button, and see "Internal server error" on a web page.

  2. BUT on the second attempt after the password and "login" button I see this page: image I check all scopes, press "login" button, but nothing happens.

In the log I see:

[INFO:CONSOLE(0)] "Refused to send form data to 'https://test-blog.joaocosta.eu/admin/indieauth' because it violates the following Content Security Policy directive: "form-action 'self'".
                                                                                                    ", source: https://test-blog.joaocosta.eu/auth?scope=read%20write%20follow&response_type=code&client_id=http%3A%2F%2Fandstatus.org&redirect_uri=http%3A%2F%2Foauth-redirect.andstatus.org&state=state_1436_1671163704135 (0)

So there is a bug on the web page preventing form submission.

yvolk commented 1 year ago

And as we see on that page "login as None" :-) No wonder: I didn't have a chance / text box to type my test login anywhere...

tsileo commented 1 year ago

Hey!

Microblog.pub developer here, I will fix the CSP issue (I have a workaround but it's not applied here, as for more context, Firefox allows redirect after a form submissions, but Chrome doesn't).

And for the "login as None", it is expected, as the server implements IndieAuth: https://indieauth.spec.indieweb.org/

Basically the main differences are:

If you’re familiar with writing an OAuth client, then you're familiar with the problem of having to register your client manually with each OAuth provider. IndieAuth uses DNS as a replacement for client registration, thereby eliminating the need for any manual registration with providers.

End-Users and Clients are all represented by URLs. IndieAuth enables Clients to verify the identity of an End-User, as well as to obtain an access token that can be used to access resources under the control of the End-User.

I am able to login with IndieAuth compatible service.

I will see how hard it is to implement dynamic client registration while keeping IndieAuth compatibility.

Thanks!

yvolk commented 1 year ago

@tsileo Thanks for working on this. Regarding Dynamic Client Registration I think that this is the most practical way when a client is a mobile app, especially an Open source app that may be built by anybody. DNS as a replacement of the app registration is meaningless here and can be easily replaced just by a simple Dynamic Client Registration stub at a server side.

I think that every instance of the app (i.e. every app launched at every users' device) should be a DIFFERENT client for a server, because nobody can be sure e.g. that two instances of the app claiming that it is "AndStatus v.X.xx" really correspond to the same running code. Yes, I remember registering two flavors of AndStatus app in Twitter about 12 years ago (that registration still works!). This is NOT about any security or reliability :-) as anybody can reuse the same key/secret in their own build of any other app. Today that way of registration is just an anachronism...

...but let's focus on making the general authentication/authorization flow used by many mobile Social apps clients work for Microblog.pub also :-)

JD557 commented 1 year ago

With this change I was finally able to get the full login flow to work 😃

For reference, I had to manually patch the server, as it is expecting the client_id to be sent on the token request as well. But maybe this is solved with the dynamic client registration.

tsileo commented 1 year ago

@yvolk I agree with your points! I should have it implemented by the end of the week.

@JD557 that's weird, it looks like Mastodon token endpoints require both client_id and client_secret: https://docs.joinmastodon.org/methods/oauth/#token

yvolk commented 1 year ago

@tsileo AndStatus may not follow Mastodon's current specs fully as Mastodon support in it was added long time ago, and it "just works" :-) If something is missed from OAuth 2.0 correctness point of view, I will gladly fix this in AndStatus.

It's interesting that when Mastodon was only being created in 2016, I tried to persuade @Gargron to adopt client to server API similar to that of Pump.io that is almost what we have in ActivityPub now, but he chose Twitter-like API at that time...

tsileo commented 1 year ago

I too think it's unfortunate that most AP servers don't support C2S. I like the idea of being able to have server that just store/process objects/activities, but I also understand it makes writing clients much more complex.

Looking at the spec, I think client_id is required when requesting a token: https://www.rfc-editor.org/rfc/rfc6749#section-4.1.3.

Also, I implemented dynamic registration, I tried to follow the RFC so the expected request looks like (https://git.sr.ht/~tsileo/microblog.pub/commit/5cf54c278221f7edf8ad1aa0517878e621f341b0#app/indieauth.py-2-8):

     {
      "redirect_uris": [
        "https://client.example.org/callback",
        "https://client.example.org/callback2"],
      "client_name": "My Example Client",
     "scope":  "read write"
     }

It looks like mastodon did not bother looking at the spec...

yvolk commented 1 year ago

@tsileo 1.

Looking at the spec, I think client_id is required when requesting a token: https://www.rfc-editor.org/rfc/rfc6749#section-4.1.3.

I use ScribeJava for OAuth2, it supports tens of real services out of the box, and it's strange that only one of them adds client_id to the Access Token Request ( "Imgur"...) https://github.com/scribejava/scribejava/blob/314d431b10a6be586668a3e1e755545a5f6a28e2/scribejava-apis/src/main/java/com/github/scribejava/apis/imgur/ImgurOAuthService.java#L28

I will add client_id anyway as it's in OAuth 2.0 spec.

  1. Regarding Client registration, looks like this was implemented in AndStatus before the spec, for Pump.io, and it works for Mastodon (maybe historically...). Will your implementation work with what AndStatus sends? (especially "redirect_uris" being a string and not an array...)
tsileo commented 1 year ago

Yes that sounds reasonable. I just pushed a change to allow both a list and string for redirect_uris.

And thanks for the client_id support!

yvolk commented 1 year ago

Hello @tsileo and @JD557 . Please check updated AndStatus v.59.12 at #456

  1. I checked that redirect_uris as an array doesn't work for mastodon.social host, I'm getting "Redirect URI can't be blank". So AndStatus makes up to two requests now, starting with RFC 7591 compliant one and in a case of an error - fallback to Mastodon specific hack.

  2. I extended Client Metadata in the Client Registration Request according to https://www.rfc-editor.org/rfc/rfc7591#section-2 and now the Client Registration Request is this:

    {
    "redirect_uris": ["http:\/\/oauth-redirect.andstatus.org"],
    "client_name": "AndStatus",
    "client_uri": "http:\/\/andstatus.org\/andstatus",
    "logo_uri": "http:\/\/andstatus.org\/images\/andstatus-logo.png",
    "scope": "read write follow",
    "policy_uri": "https:\/\/github.com\/andstatus\/andstatus\/blob\/master\/doc\/Privacy-Policy.md"
    }
  3. Added client_id to the Access Token Request as we discussed.

Using test account I could log in: image

And was successfully navigated to the AndStatus app's Home timeline (no notes here though...) image

JD557 commented 1 year ago

To me it looks like it was working (tested both on the test and my personal service). 👍

Thanks for this.

yvolk commented 1 year ago

Good. Waiting for the news about successful reading and publishing to Microblog.pub via AndStatus :-)

tsileo commented 1 year ago

Thanks a lot!!

I need to do some work on the C2S APIs, microblog.pub used to have it but for the v2 there's only partial support.

I'll start working on it soon and I'll report back here.

tsileo commented 1 year ago

Screenshot_20221218-104446

Reading seems to work with my development version! :tada:

tsileo commented 1 year ago

@yvolk do you support refreshing access tokens? I tried to add support for it (returning a refresh_token+ supporting the refresh_token grant type on the token endpoint), but nothing happens when an endpoint returns a 401.

yvolk commented 1 year ago

No, didn't need refresh tokens yet :-) In a case of similar problems a User needs to go to Settings -> Accounts -> Manage accounts -> select an account and press "Reverify credentials" button...

tsileo commented 1 year ago

Thanks!

I do have to log out/re-login when it happens. Reverify credentials is just pinging / which is public/not-authenticated, so it does not detect that a login is needed.

I was able to post a note, but I noticed it expects the AP object in the response body, I assumed reading the spec that only the AP ID is expected in the Location header:

Example 12: Outbox response to submitted Activity

HTTP/1.1 201 Created
Location: https://dustycloud.org/likes/345

I guess it should not disturb clients not expecting a body (and I may misinterpret the spec).

yvolk commented 1 year ago

Yes, reading this section of the specification I see that formally the spec does NOT require anything in the response except for the id in the Location header. :astonished: Strange, but looking into the code of AndStatus I see that in existing implementation in AndStatus I didn't implement a case when id in the Location header is the only "content" of the response. Previous known implementations did return an Activity in the response.

But this is not a problem to add. Generally AndStatus can handle object initially known by their id only. As long as I can reproduce this at the test microblog.pub instance, I will add this. But probably what I add will be another get request to the location just in order to know ALL Activity properties, generated by a server. We could think about optimization later...

yvolk commented 1 year ago

@tsileo If you are ready to implement at least some more parts of the spec, should we create a separate ticket for that here?! I see that I will need to make more changes also...

tsileo commented 1 year ago

Sure, everything looks good for /.well-known/oauth-authorization-server support, thanks again!

And yes, that sounds good! I've already pushed some read/write support via C2S:

Do you want me to open tickets? I am thinking about:

tsileo commented 1 year ago

@JD557 if you update your test instance, it should have basic read/write support.

yvolk commented 1 year ago
  1. Regarding rationale of getting more than Activity id only from server: 1.1. During Create Activity at least TWO objects created: an Activity and a Note. So we need id of the Note anyway. 1.2. Knowing object's "published" and "updated" timestamps are important in knowing, when we need to update an object in the local cache (this is how AndStatus works...)

  2. Yes, please create tickets as you notice most important gaps... (so we could do most important things first...). These two are good, thank you.

  3. Regarding dumb AndStatus reaction on authentication errors this needs to be improved anyway. And needs a discussion probably...

tsileo commented 1 year ago

Thank you!

JD557 commented 1 year ago

Hey @tsileo, just tried and it seems to be mostly working. Some notes:

Anyway, I'll also use this comment to "sign-off" from this issue. I might check it from time to time, but due to holidays I won't have regular access to a dev machine for quite some time.

yvolk commented 1 year ago

Hi @JD557 and @tsileo, In this ticket we solved problems that blocked all other ActivityPub-related possible improvements and fixes. Thank you! So I'm closing this ticket as done and will work on others, step by step. Hope not to disappear for a long time :-)

yvolk commented 1 year ago

As a follow up to the improvement of our apps' ActivityPub C2S support I created a couple of tickets in microblog.pub Bug Tracker. Linking to them here to allow discovery: