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

Support for SAML 1.1 tokens #4

Merged
merged 3 commits into from
Dec 11, 2017
Merged

Support for SAML 1.1 tokens #4

merged 3 commits into from
Dec 11, 2017

Conversation

brat000012001
Copy link

Before merging my changes it would be great to have the changes reviewed by other people. The changes include the support for SAML 1.1 tokens, the changes to the admin console interfaces to allow users configure a WS-Fed client, a few minor changes to the Ws identity provider to automatically pick up the brokered user's last name, first name and the user email;unit tests to verify SAML 1.1 tokens

… format, i.e. jwt, saml 2.0 or saml 1.1) via the admin console

Support for SAML 1.1 assertion tokens; one of the identity provider tests  is failing and needs work (everything seems to pass but the expected IdP link does not match the expected value, possibly the unit test issue)

 Please enter the commit message for your changes. Lines starting
@brat000012001 brat000012001 changed the title Changes to allow users to configure the Ws-Fed client settings (toke… Support for SAML 1.1 tokens Oct 21, 2017
@brat000012001 brat000012001 self-assigned this Oct 23, 2017
@braoru
Copy link

braoru commented Oct 23, 2017

Hello,

Thanks for the contribution, we're doing our best to review it !

We are currently working on the migration fo the plugin to the latest version of keycloak. I'll try to merge the two task in one :-)

Thanks,

Sébastien

@brat000012001
Copy link
Author

Sebastien,
Keep in mind that the changes to the freemarker templates and .js client code are specific to 2.5.5 version of KC. I am also hoping to get some input from the original implementers (Dan Barentine at el) .
Best,
--Peter

@AlistairDoswald
Copy link

@brat000012001 I'll try to review the code and test it functionally this week. I'll should be able to post my findings here during this week or the next.

@AlistairDoswald
Copy link

A small update: I've tested functionally the simple Web case "browser - STS - WS Resource" with "browser - Keycloak - SharePoint", and so far everything works out for a simple login/logout scenario (aside from one missing import - javax.api - and the "client>Installation" page which is missing/garbled). The structure of the SAML 1.1 token also checks out against those produced by ADFS. I hope to be able to check out more complicated scenarios - including the ApplicationServiceType - next week.

@brat000012001
Copy link
Author

brat000012001 commented Oct 29, 2017

Hi Alastair,
Thanks for testing this. I am aware of the installation page - it should be relatively simple to to fix it, just didnt get around to it.

@AlistairDoswald
Copy link

Hi Peter,

It took me longer than expected, due to other projects taking more time than expected, but I've managed to continue testing your code. This time I've been testing the broker side of the code with the following setup:

browser (requestor) <--> sharepoint (resource)<--> keycloak as identity broker <--> keycloak as IDP

This is actually a setup in the same pattern as in section 13.3 of the ws-fed specification (http://docs.oasis-open.org/wsfed/federation/v1.2/os/ws-federation-1.2-spec-os.html), which has the advantage of having expected messages to be described in appendix B of the same specification, which allowed me to compare what was going on.

The result is that I was able to check that your modifications to the code work, and also check the attached code. As a result, we'll be validating the pull request very soon.

However, I wasn't able to make the full example work. Going back to the section section 13.3 of the specification, at step 8 I'm getting a 302 instead of an POST (form) or an error page while using the "browser" first flow, and a nullpointer when using first broker login flow. If you have been working with keycloak as broker and got it to work, I'd be interested in your insights.

@brat000012001
Copy link
Author

Hey Alistair,
Thanks for testing things out. I will take a look at the issue you are running into. Do not worry about merging the pull request, i can do it myself once we are confident that everything is working as expected.

@AlistairDoswald
Copy link

Hi Peter,
If you have no objections, I'm going to merge your pull request and add the two problems I've seen (installation page and broker) as issues. I've completed the modifications to migrate the keycloak ws-fed to version 3.4.0.Final of Keycloak, but I've done it in a local branch which is branched from the ws-fed11 branch. The idea is to get all code into the master ASAP, and do any necessary corrections from there. I'll also tag the code (current master + ws-fed11) as 2.5.5.Final before adding my migration to 3.4.0.Final code.

@brat000012001
Copy link
Author

@AlistairDoswald No, no objections.

@AlistairDoswald AlistairDoswald merged commit 68ce02b into master Dec 11, 2017
@AlistairDoswald AlistairDoswald deleted the ws-fed11 branch December 11, 2017 16:04
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.

3 participants