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

modified to serve as an OAuth2 introspection endpont #15

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

carrvo
Copy link

@carrvo carrvo commented Dec 3, 2024

See IndieAuth spec for more information about the response that needs to be returned by the introspection endpoint.

@carrvo carrvo mentioned this pull request Dec 3, 2024
Copy link
Owner

@Zegnat Zegnat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be really nice to give Mintoken support for Introspection, nice! There are a couple of things I am not too sure about, because I haven’t thought it through.

Is it OK to use the same endpoint for all other actions and token introspection? With this change, just depending on the POST body including a token the token endpoint behaviour switchtes completely into acting like an introspection endpoint. It might be better to introduce e.g. a URL-query string to split behaviour with.

As IndieAuth introduced a MUST for authorization towards this endpoint that would need some thought as well. The easiest for Mintoken to support would be Bearer token based. It could issue a token itself, maybe with a specific predetermined scope (mintoken:introspection?). Then it will allow introspection only if a token with that scope is provided. Thoughts?

Also putting some code comments here, in case you want to roll this out in your own version but were interested in the code.

My implementations tend to be very spec-first.

endpoint.php Outdated Show resolved Hide resolved
endpoint.php Show resolved Hide resolved
endpoint.php Outdated Show resolved Hide resolved
endpoint.php Outdated Show resolved Hide resolved
endpoint.php Show resolved Hide resolved
endpoint.php Outdated Show resolved Hide resolved
@carrvo
Copy link
Author

carrvo commented Dec 3, 2024

It would be really nice to give Mintoken support for Introspection, nice! There are a couple of things I am not too sure about, because I haven’t thought it through.

Is it OK to use the same endpoint for all other actions and token introspection? With this change, just depending on the POST body including a token the token endpoint behaviour switchtes completely into acting like an introspection endpoint. It might be better to introduce e.g. a URL-query string to split behaviour with.

As IndieAuth introduced a MUST for authorization towards this endpoint that would need some thought as well. The easiest for Mintoken to support would be Bearer token based. It could issue a token itself, maybe with a specific predetermined scope (mintoken:introspection?). Then it will allow introspection only if a token with that scope is provided. Thoughts?

Also putting some code comments here, in case you want to roll this out in your own version but were interested in the code.

My implementations tend to be very spec-first.

I think this comes down to organization. I had thought that you had designed this to be a standalone file that could be downloaded and work on its own. You already have revoking on the same endpoint differentiated by the body, so it seemed to align as well. I am completely ok with whatever direction you think is suitable, but here are a few brainstorming ideas:

  • POST differentiated by body (what I did)
  • POST differentiated by query (what you suggest)
  • endpoints differentiated by URI - I don't like this because the script would have to assume what the endpoint is called
  • separate endpoints differentiated by separate files - for best code reuse this would become three files, one imported/required by the other two

Obviously I haven't dived into the client/resource authenticating itself yet so I am quite open. My thoughts were to do something that would align with the idea that clients don't have to register beforehand (just like they don't when signing the user in), which is what attracted me to IndieAuth in the first place. So I was leaning towards either:

  • using the token itself (easy to verify the token) and checking that it has the right scope--which is inspired by OIDC--but it is not simple to configure mod_oauth2 with;
  • or, my better idea, is Basic with just the client ID (URI) and we can filter the results such that it can only introspect those that are meant to belong with it. Aka, WHERE token_id = ? AND client_id = ? exec([split_id_from_token($token), split_user_from_basic($_SERVER['Authorization'])) as a rough pseudo-code (and wrong names).

I agree with spec-first! I just haven't read much of it and only implement to the best of what I have read and understood (I see from your other comments things that I have yet to come across).

@carrvo
Copy link
Author

carrvo commented Dec 4, 2024

You already have revoking on the same endpoint differentiated by the body, so it seemed to align as well.

I have not dived into it or tried it, but it seems that the spec is expecting it as a separate endpoint with different parameters than you: https://indieauth.spec.indieweb.org/#token-revocation
So it may be better to split these into different files that can be different endpoints more easily.

endpoint.php Outdated
Comment on lines 276 to 286
// Authorize resource server as per specification (see https://indieauth.spec.indieweb.org/#access-token-verification-response-p-1)
// For us this means we are expecting the Basic user to be the client ID of the consumer.
// With basic, everything after the first colon (:) is considered the password.
// Since we are working with URIs to identify, we need to handle
// the pattern scheme://domain/path:password
// To do this, we assume (and enforce) that the password is a single underscore (_).
if ($tokenInfo['auth_client_id'] . ':_' !== $_SERVER['PHP_AUTH_USER'] . ':' . $_SERVER['PHP_AUTH_PW']) {
header('WWW-Authenticate: Basic');
header('HTTP/1.0 401 Unauthorized');
exit('Unauthorized');
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to additionally check if a pre-defined scope is present in the token itself. Similar to how OIDC requires the openid scope.

@Zegnat
Copy link
Owner

Zegnat commented Dec 4, 2024

You already have revoking on the same endpoint differentiated by the body, so it seemed to align as well.

I have not dived into it or tried it, but it seems that the spec is expecting it as a separate endpoint with different parameters than you

Correct. Mintoken was based on an older version of the IndieAuth spec. Token revocation was implemented as per the November 2020 edition.

Modern version of IndieAuth has aligned itself more with OAuth through Authorization Server Metadata (RFC 8414). That allows the authorization server to specify a bunch of different endpoints at different addresses.

I had thought that you had designed this to be a standalone file that could be downloaded and work on its own.

Also correct. Therefor my first thought was to make the file switch roles based on a URL-query string. That would also be relatively easy for people to do if they want to create the Server Metadata file: you would put the same link pointing at this PHP file everywhere and simply add a string at the end.


I need to have another pass over how best to handle authentication. Do you have any documentation from mod_oauth2 what is supported there? Happy to take that into consideration as well.

I am not a big fan of a hard-coded client secret (_) 😅 Client IDs, especially in the IndieAuth ecosystem, are not secret. So if just knowing a client ID is enough to gain access to introspection it is basically the same as having no authentication at all.

@carrvo
Copy link
Author

carrvo commented Dec 4, 2024

You already have revoking on the same endpoint differentiated by the body, so it seemed to align as well.

I have not dived into it or tried it, but it seems that the spec is expecting it as a separate endpoint with different parameters than you

Correct. Mintoken was based on an older version of the IndieAuth spec. Token revocation was implemented as per the November 2020 edition.

Modern version of IndieAuth has aligned itself more with OAuth through Authorization Server Metadata (RFC 8414). That allows the authorization server to specify a bunch of different endpoints at different addresses.

I had thought that you had designed this to be a standalone file that could be downloaded and work on its own.

Also correct. Therefor my first thought was to make the file switch roles based on a URL-query string. That would also be relatively easy for people to do if they want to create the Server Metadata file: you would put the same link pointing at this PHP file everywhere and simply add a string at the end.

Ultimately this is your project so I am fine to try out query parameters and let you know of any issues I come across with the libraries that I am using.

I need to have another pass over how best to handle authentication. Do you have any documentation from mod_oauth2 what is supported there? Happy to take that into consideration as well.

I am not a big fan of a hard-coded client secret (_) 😅 Client IDs, especially in the IndieAuth ecosystem, are not secret. So if just knowing a client ID is enough to gain access to introspection it is basically the same as having no authentication at all.

Unfortunately that is true. I think that can be mitigated (and I forgot to make a note of this for myself) by some mTLS or mechanism to compare the requesting server's certificate against the client ID provided...but I have no idea how to do that.

I am hoping we can come up with a solution where a new client can connect with the existing IndieAuth flow and have MinToken "just work".

@carrvo
Copy link
Author

carrvo commented Dec 4, 2024

I need to have another pass over how best to handle authentication. Do you have any documentation from mod_oauth2 what is supported there? Happy to take that into consideration as well.

mod_oauth2 config
The options that I got working (without endpoint authorization) are

		AuthType oauth2
                # verify through metadata endpoint - preferred and ultimately resolves to the second option
		OAuth2TokenVerify metadata http://testapache.local/.well-known/selfauth.json metadata.ssl_verify=false&introspect.auth=client_secret_basic&client_id=http://testapache.local/oauth/&client_secret=_
                # verify through introspection
		OAuth2TokenVerify introspect http://testapache.local/selfauth/token.php introspection.ssl_verify=false&introspect.auth=client_secret_basic&client_id=http://testapache.local/oauth/&client_secret=_

This is all testing and localhost of course...

Note that I am combining the changes from here and #14

Comment on lines +276 to +281
// Authorize resource server as per specification (see https://indieauth.spec.indieweb.org/#access-token-verification-response-p-1)
// For us this means we are expecting the Basic user to be the client ID of the consumer.
// With basic, everything after the first colon (:) is considered the password.
// Since we are working with URIs to identify, we need to handle
// the pattern scheme://domain/path:password
// To do this, we assume (and enforce) that the password is a single underscore (_).
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also verify that the request is from the client specified, checking their certificate to do so.

endpoint.php Show resolved Hide resolved
@carrvo
Copy link
Author

carrvo commented Dec 4, 2024

You already have revoking on the same endpoint differentiated by the body, so it seemed to align as well.

I have not dived into it or tried it, but it seems that the spec is expecting it as a separate endpoint with different parameters than you

Correct. Mintoken was based on an older version of the IndieAuth spec. Token revocation was implemented as per the November 2020 edition.
Modern version of IndieAuth has aligned itself more with OAuth through Authorization Server Metadata (RFC 8414). That allows the authorization server to specify a bunch of different endpoints at different addresses.

I had thought that you had designed this to be a standalone file that could be downloaded and work on its own.

Also correct. Therefor my first thought was to make the file switch roles based on a URL-query string. That would also be relatively easy for people to do if they want to create the Server Metadata file: you would put the same link pointing at this PHP file everywhere and simply add a string at the end.

Ultimately this is your project so I am fine to try out query parameters and let you know of any issues I come across with the libraries that I am using.

Query parameters works with mod_oauth2 ;)

@Zegnat
Copy link
Owner

Zegnat commented Dec 5, 2024

I need to have another pass over how best to handle authentication. Do you have any documentation from mod_oauth2 what is supported there? Happy to take that into consideration as well.

mod_oauth2 config

Thanks, very interesting! It looks like their Authentication Options mostly mirror the IANA registry for OAuth Token Endpoint Authentication Methods. I am surprised they do not support token based authentication, as introspection_endpoint_auth_methods_supported also allows values from the IANA registry for OAuth Access Token Types (e.g. Bearer).

I would have prefered to implement Bearer token authentication. But it might just be that a different method should be included. I do think there should be a secret involved. Looking at those supported by mod_oauth2, it seems like it would be client_secret_basic or client_secret_post. I would like to steer clear of introducing JWT/JWK checking and certificates. basic seems to not be a valid method in the OAuth spec.

Lots of food for thought, thank you for sticking with it and I apologise for the long debate. I would like to keep Mintoken bth simple and close to the specs. Easy to work hours on comparing specs by myself, harder to do in a GitHub issue 😉

@carrvo
Copy link
Author

carrvo commented Dec 5, 2024

I would have prefered to implement Bearer token authentication. But it might just be that a different method should be included. I do think there should be a secret involved. Looking at those supported by mod_oauth2, it seems like it would be client_secret_basic or client_secret_post. I would like to steer clear of introducing JWT/JWK checking and certificates. basic seems to not be a valid method in the OAuth spec.

I think I understand the desire for a Bearer token (more secure for sure), but I suspect that would mean clients have to pre-register and then be able to send a different token based upon which provider is chosen. While registering is the norm for OAuth (and continued in OIDC), personally I have found that registering invalidates internet posts claiming OAuth is "quick and easy to setup". And it adds a level of complexity you want to stay away from.

I do not know what the difference is between basic and client_secret_basic, but I have mine set to the latter and it is working. 'Course I don't have a real shared secret and I don't know how we could verify a secret without, again, pre-registering every client.

Lots of food for thought, thank you for sticking with it and I apologise for the long debate. I would like to keep Mintoken bth simple and close to the specs. Easy to work hours on comparing specs by myself, harder to do in a GitHub issue 😉

I am super happy for the long debate! I know very little about the spec and it shows that you are taking me seriously anyway! I am very grateful that you are interested in my proposed changes and have been regularly checking in on my progress. I would prefer keeping the baseline implementation where you can keep the credit, and not in some derivation/fork that I hold. If you think it would be easier, I would be open to an online meetup ( I assume after the New Year) so we can discuss it directly instead of through GitHub. I really look forward to using MinToken!

I desire to keep it simple as well; and am working towards a solution that can get non-technical people using IndieAuth within an hour or so (and, yes, I indieauth.com gives a quick provider but I would like an isolated alternative). Besides, it is always good to keep a simple solution in the back pocket when there are lots of more sophisticated solutions out there. I just made a short blurb that gives a bit more background on where I am coming from in case it helps discussions like these: http://turner.enemyterritory.org/HomeCloud

}
header('HTTP/1.1 200 OK');
header('Content-Type: application/json;charset=UTF-8');
exit(json_encode([
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also markTokenUsed like in the GET/consumption endpoint.

@carrvo
Copy link
Author

carrvo commented Dec 22, 2024

For the Authentication of the introspection the IndieWeb developers had some feedback (I forgot to duplicate it here)

[schmarty]
the introspection endpoint is not for indieauth clients. in oauth terms, it's for "resource servers" - things like your micropub server that will receive a token from a client and need to verify that the token is valid, who it belongs to, what scopes it was granted, what client requested it, etc.
it's expected that the indieauth server and resource servers know about each other, allowing some kind of authenticated access to the introspection endpoint.
aaronpk has more details from the oauth perspective: https://www.oauth.com/oauth2-servers/the-resource-server/

And from the link

The token introspection endpoint is intended to be used only internally

So it looks like the intended purpose is internally/registered, despite not needing this "know each other" elsewhere/earlier.

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

Successfully merging this pull request may close these issues.

2 participants