Add documentation for Jetty OpenID Connect support #12559
Conversation
Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
| public void createConfigurationWithDiscovery() | ||
| { | ||
| // tag::createConfigurationWithDiscovery[] | ||
| OpenIdConfiguration openIdConfig = new OpenIdConfiguration(ISSUER, CLIENT_ID, CLIENT_SECRET); |
There was a problem hiding this comment.
Can you make examples for these constants?
For example:
String issuer = "<example-issuer>";
...
OpenIdConfiguration openIdConfig = new OpenIdConfiguration(issuer, clientId, clientSecret);| public void configureLoginService() | ||
| { | ||
| // tag::configureLoginService[] | ||
| LoginService loginService = new OpenIdLoginService(openIdConfig); | ||
| securityHandler.setLoginService(loginService); | ||
| // end::configureLoginService[] | ||
| } | ||
|
|
||
| public void configureAuthenticator() | ||
| { | ||
| // tag::configureAuthenticator[] | ||
| Authenticator authenticator = new OpenIdAuthenticator(openIdConfig, "/error"); | ||
| securityHandler.setAuthenticator(authenticator); | ||
| // end::configureAuthenticator[] | ||
| } |
There was a problem hiding this comment.
Feels like these should not be split.
Would be better if you can write a full snippet of code that would work in the most common case.
Optional configuration can be explained as a code comment in the snippet.
| private LoginService createWrappedLoginService() | ||
| { | ||
| HashLoginService loginService = new HashLoginService(); | ||
| UserStore userStore = new UserStore(); | ||
| userStore.addUser("admin", Credential.getCredential("password"), new String[]{"admin"}); | ||
| loginService.setUserStore(userStore); | ||
| loginService.setName(ISSUER); | ||
| return loginService; | ||
| } |
There was a problem hiding this comment.
This code seems important, but it is not showed in the documentation.
|
|
||
| ---- | ||
| include::{jetty-home}/modules/openid.mod[tags=documentation] | ||
| ---- |
There was a problem hiding this comment.
Have a minimal discussion about the most important, or mandatory, properties.
See for example the http module in this file.
|
|
||
| To configure OpenID Authentication with Jetty you will need to specify the OpenID Provider's issuer identifier (case-sensitive URL using the `https` scheme) and the OAuth 2.0 Client ID and Client Secret. | ||
| If the OpenID Provider does not allow metadata discovery you will also need to specify the token endpoint and authorization endpoint of the OpenID Provider. | ||
| These can be set as properties in the `start.ini` or `start.d/openid.ini` files. |
There was a problem hiding this comment.
Please make an example of the *.ini file.
| [[openid-support]] | ||
| = OpenID Support | ||
|
|
||
| == External Setup |
There was a problem hiding this comment.
What's this empty section?
What exactly means "external" here?
This is the place where we want to have a link to Wikipedia, and describe OpenID in a little more details.
If an external setup is a required setup, it should be described as such, like "OpenID requires you to configure an external identity provider, in addition to local configuration. To configure the external identity provider, you need ..."
| === Registering an App with OpenID Provider | ||
| You must register the app with an OpenID Provider such as link:https://developers.google.com/identity/protocols/OpenIDConnect[Google] or link:https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc.html[Amazon]. | ||
| This will give you a Client ID and Client Secret. | ||
| Once set up you must also register all the possible URI's for your webapp with the path `/j_security_check` so that the OpenId Provider will allow redirection back to the webapp. |
There was a problem hiding this comment.
This is not clear? Isn't there just one URI, that ends with /j_security_check?
If there is more than one, needs a more elaborate example, such as: "Let's assume your web application deploys at +https://example.com+; then the URIs are ..."
|
|
||
| === Define the `OpenIdConfiguration` for a specific OpenID Provider. | ||
|
|
||
| If the OpenID Provider allows metadata discovery then you can use. |
There was a problem hiding this comment.
These examples are largely incomplete, as users would have no idea what to do with an instance of OpenIdConfiguration.
It would be better to have a full example first for the case of metadata discovery, that shows the code that allocates the Server, the ServerConnector, the OpenIdConfiguration, the LoginService, etc. all what's needed.
Then in the other sections you can just refer to the example saying: "In this case, the OpenIdConfiguration should be created in this way: ".
|
|
||
| If security roles are required they can be configured through a wrapped `LoginService` which is deferred to for role information by the `OpenIdLoginService`. | ||
|
|
||
| This can be configured in XML through `etc/openid-baseloginservice.xml` in the Distribution, or in embedded code using the constructor for the `OpenIdLoginService`. |
There was a problem hiding this comment.
This needs to be split: the section about operation in the operations guide, and the section about code here.
| include::code:example$src/main/java/org/eclipse/jetty/docs/programming/security/OpenIdDocs.java[tags=wrappedLoginService] | ||
| ---- | ||
|
|
||
| When using authorization roles, the setting `authenticateNewUsers` becomes significant. |
There was a problem hiding this comment.
What class has authenticateNewUsers?
This is the programming guide, if this is important in the operations guide, then must be specified there.
replaces #12234