Skip to content

SAML AuthNRequest Signatures - Step 1 #7758

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 12, 2020
Merged

SAML AuthNRequest Signatures - Step 1 #7758

merged 1 commit into from
Feb 12, 2020

Conversation

fhanik
Copy link
Contributor

@fhanik fhanik commented Dec 20, 2019 •
edited
Loading

Simple fix for gh-7711 that changes the way the AuthNRequest is signed and

Has been tested with

  • Keycloak
  • SSOCircle
  • Okta
  • SimpleSAMLPhp

Further configuration options (POST vs REDIRECT) that build on top of this PR can be found in:
#7759

@fhanik fhanik added status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged in: saml2 An issue in SAML2 modules labels Dec 20, 2019
@fhanik fhanik self-assigned this Dec 20, 2019
This was referenced Dec 20, 2019
Merged
Closed
@fhanik fhanik force-pushed the issue/7711 branch 5 times, most recently from 6515265 to d6501a3 Compare January 2, 2020 04:38
@fhanik fhanik removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Jan 2, 2020
@fhanik fhanik added this to the 5.3.0.RC1 milestone Jan 2, 2020
@fhanik fhanik requested a review from rwinch January 7, 2020 15:54
@fhanik fhanik added the type: bug A general bug label Jan 7, 2020
@fhanik fhanik changed the title SAML AuthNRequest Signatures - Option 1 SAML AuthNRequest Signatures - Step 1 Jan 10, 2020
@rwinch rwinch requested a review from jzheaux January 14, 2020 16:27
jzheaux
jzheaux requested changes Jan 27, 2020
View reviewed changes
jzheaux
jzheaux requested changes Jan 30, 2020
View reviewed changes
Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Great progress, @fhanik. I've left some additional feedback inline.

jzheaux
jzheaux requested changes Feb 7, 2020
View reviewed changes
Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Great progress, @fhanik! I've left just a bit more feedback inline.

jzheaux
jzheaux requested changes Feb 10, 2020
View reviewed changes
Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Nice, @fhanik! I've left some additional feedback.

@fhanik fhanik force-pushed the issue/7711 branch 7 times, most recently from ceeec72 to 0c38887 Compare February 11, 2020 20:54
jzheaux
jzheaux requested changes Feb 11, 2020
View reviewed changes
Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Looking good, @fhanik. I've left some additional feedback. Also, in preparation for merging, will you please format your commit message to follow the contribution guidelines?

@fhanik fhanik force-pushed the issue/7711 branch 3 times, most recently from ab8f204 to c69833d Compare February 12, 2020 00:23
@fhanik
Correct signature handling for SAML2 AuthNRequest
96545ca 
Implements the following bindings for AuthNRequest
- REDIRECT
- POST (future PR)

Has been tested with
- Keycloak
- SSOCircle
- Okta
- SimpleSAMLPhp

Fixes spring-projectsgh-7711
@jzheaux
Copy link
Contributor

jzheaux commented Feb 12, 2020

Nice job, @fhanik! Before you merge, please squash your commits. Also, please consider updating the other .pdf URLs to use an <a> tag like the one you changed. Otherwise, LGTM.

@fhanik fhanik merged commit a3e09fa into spring-projects:master Feb 12, 2020
fhanik added a commit to fhanik/spring-security that referenced this pull request Feb 28, 2020
@fhanik
Support POST binding for AuthNRequest
f460ea5 
Has been tested with

- Keycloak
- SSOCircle
- Okta
- SimpleSAMLPhp

This PR extends (builds on previous commits and adds user configuration
options)
spring-projects#7758
fhanik added a commit that referenced this pull request Feb 28, 2020
@fhanik
Support POST binding for AuthNRequest
3257349 
Has been tested with

- Keycloak
- SSOCircle
- Okta
- SimpleSAMLPhp

This PR extends (builds on previous commits and adds user configuration
options)
#7758
@jzheaux jzheaux added status: duplicate A duplicate of another issue type: enhancement A general enhancement and removed type: bug A general bug labels Mar 4, 2020
jzheaux added a commit that referenced this pull request Sep 26, 2020
@jzheaux
Polish OpenSamlAuthenticationRequestFactory
a36baff 
- Refactored to use SAMLMetadataSignatureSigningParametersResolver

Issue gh-7758
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwinch rwinch rwinch requested changes

@jzheaux jzheaux jzheaux approved these changes

Assignees

@fhanik fhanik

Labels
in: saml2 An issue in SAML2 modules status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Milestone
5.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@fhanik @jzheaux @rwinch @eleftherias