KEYCLOAK-2469 - Introduced new redirect endpoint for clients.

[thomasdarimont]

Previously one had to configure hardcoded urls to link from one client
application to others since keycloak didn't provide a way to get the
actual client URL by providing clientId and realm information.

We now support a new endpoint with the path {realm}/clients/{client_id}/redirect
that responds to GET requests with a 307 (temporary redirect) with the
configured client URL. This allows to refer to any client just by the
realmName and clientId and let Keycloak send a redirect to the actual client
application.

[stianst]

This is the same path as the client-registration service is registered at. We either need a different path for this or move the client-registration. With regards to having a clients landing page like we discussed in mailing list it makes more sense to move client registration service, but that means changing endpoints that are already included in past releases. I'll figure out what to do here.

[thomasdarimont]

I saw that the path of the client-registrations endpoint was changed, is the
{realm}/clients/{client_id}/redirect fine then?

[stianst]

Yep

[stianst]

We'll have to wait until 1.9.x branch is created before merging this. We can't accept new features into 1.9.x.

[thomasdarimont]

Thanks for the review :)
I adapted the construction of the relative URI with your suggested approach.

[thomasdarimont]

How can I parameterize the keycloak port here? Just via: System.getProperties("keycloak.port") or via System.getenv("KEYCLOAK_DEV_PORT")?

[stianst]

I missed that. New tests should be added to testsuite/integration-arquillian.

[thomasdarimont]

Okay, I try to adapt that - though I've some issues:

• How can I create the two test clients within the test realms? The KeycloakServer (as well as WebDriver, WebRule) rule isn't available to the arquillian-integration tests
• Do I need to create "Page" to test the redirect?

I couldn't find an example that does something similar in arquillian-integration tests - or did I miss something? Any hints?

[stianst]

Extend AbstractKeycloakTest and use adminClient to create via REST endpoints.

You can either create "Page" or just use driver.getCurrentUrl(), up to you.

[thomasdarimont]

Thanks I made some progress with this - will push the new test cases soon, though the initial tests were much easier to write...

[stianst]

DO NOT MERGE UNTIL 1.9.x IS CREATED

[stianst]

This needs some documentation otherwise it'll just be a hidden feature. @thomasdarimont can you take care of that?

[thomasdarimont]

@stianst sure - can you give me some pointers where I should pace the documentation - which chapter/sub-chapter etc.

[stianst]

I can't see any where obvious to add it, so I'd suggest adding a new chapter "Clients" with a sub-section "Redirect". We can re-organize other parts of the docs that are related to clients under there.

[thomasdarimont]

Ok thanks for the hint - next question, since the readme doesn't mention it - how do I actually build the docs (I guess the relevant files are in docbook/auth-server-docs ...)?

Would be good to have a section for that in the misc/HackingOnKeycloak.md doc.

[stianst]

cd docbook
mvn clean install

or

mvn clean install -Pjboss-release

[thomasdarimont]

Added section for the docs - but I haven't time to move the tests :-/

[stianst]

@thomasdarimont master branch is finally ready to accept new features for 2.x. Any chance you could rebase this? I'd also love it if you could port the test to the new Arquillian based testsuite (testsuite/integration-arquillian/tests/base).

[thomasdarimont]

Thanks for having another look at this - I'll do so ASAP :)

[thomasdarimont]

@stianst I rebased the PR on the current master and squashed the individual commits into one.

The rebasing took me 15 mins... then I tried to move the tests over to the arquillian-integration-tests but after 90mins I gave up... a few comments here and there would help a lot...

In my test I actuall do just the following:

• create 2 tests clients, 1 client with an absolute base url, another client with a relative base URL.
• in the test I just check whether the new redirect endpoint gives me a correct HTTP 307 temporary redirect with the appropriate location.

     */
    @Test
    public void testClientRedirectEndpoint() throws Exception {

        oauth.doLogin("test-user@localhost", "password");

        webDriver.get("http://localhost:" + getKeycloakPort() + "/auth/realms/test/clients/launchpad-test/redirect");
        assertEquals("http://example.org/launchpad", webDriver.getCurrentUrl());

        webDriver.get("http://localhost:" + getKeycloakPort() + "/auth/realms/test/clients/dummy-test/redirect");
        assertEquals("http://example.org/dummy/base-path", webDriver.getCurrentUrl());

        webDriver.get("http://localhost:" + getKeycloakPort() + "/auth/realms/test/clients/account/redirect");
        assertEquals("http://localhost:" + getKeycloakPort() + "/auth/realms/test/account", webDriver.getCurrentUrl());
    }

I tried to find a tests that does a similar thing, logging in, creating 2 clients and requesting that but it seems that the infrastructure which I used in my initial test is not available since you either work with the admin-client or "Page" instances - should I create a "Redirect Page" to check the redirect - is this how it's supposed to work?

[stianst]

@thomasdarimont if you extend org.keycloak.testsuite.AbstractKeycloakTest you'll have the adminClient that you can use to create the clients. We haven't migrated the old testsuite yet so there's a lack of features/utilities available in the new testsuite. However, I think if you just login to the account management console to login the user, then you can try the redirect to clients after that (look at org.keycloak.testsuite.account.AccountTest). If it's to much work to do, don't worry we can migrate the tests later, but eventually we want all tests in the new Arquillian based testsuite.

[stianst]

@thomasdarimont PR looks good, I'll do some minor changes and merge when I get time. It'll be a week or two though, but you can leave it with me and I'll make sure it'll get merged. Thanks

[stianst]

Closing this PR, as I'm opening another one with some minor changes

[stianst]

#2399