SEC-935: Add support for OpenID attribute exchange #1191

Closed
spring-issuemaster opened this Issue Jul 23, 2008 · 21 comments

1 participant

@spring-issuemaster

Tatyana Tokareva (Migrated from SEC-935) said:

Simple registration extension for openid login is not currently supported, so that info about user is sent from provider to relaying party.
we could make OpenidAuthenticationFilter return not just a username url, but url with params.

I already have some workaround in that and could make a patch

@spring-issuemaster

Matthias Quasthoff said:

We created a patch for Spring Security OpenID to allow for Simple Registration and Attribute Exchange. The patch will also be reflected in another patch to the Grails Acegi Plugin.

For us, this issue is related to SEC-931 as in order to use Simple Registration or Attribute Exchange, getPrincipal() must return a OpenIDUser object instead of only the identity URI (at least, this is how we implemented it)

@spring-issuemaster

Tatyana Tokareva said:

Where can i check out code to apply this patch?

@spring-issuemaster

Ray Krueger said:

Was this patch generated against a specific tag?
I’m getting errors trying to apply it due to changes that were made after the patch was generated.

@spring-issuemaster

Tatyana Tokareva said:

I’ve checked out code from the project trunk, but I failed to apply patch

@spring-issuemaster

Luke Taylor said:

The patch appears to have some problems and requires a snapshot version of openid4java which isn’t suitable for a 2.0.4 release. Changing to 2.1.

It also contains a lot of unnecessary re-formatting of existing code which isn’t ideal.

@spring-issuemaster

Christopher Schuster said:

It seems like the “sreg.patch” has some serious problems. I have cleaned the patch and uploaded it as “attribute-exchange”. The new one does not need Java 5, the formatting issues were solved and it can be applied to the current trunk of the “/openid/” folder.

Three problems remain:
1) the mechanisms for AttributeExchange depend on a new version of OpenId4Java. (at least 0.9.5)
2) the TestCases are far from complete.
3) the OpenId4JavaConsumer probably lacks support for concurrent authentications.

@spring-issuemaster

Christopher Schuster said:

This Patch depends on OpenID4Java 0.9.5.

Projects using Spring Security can fetch User Details from the OpenIDProvider by configuring the OpenIDConsumer. After a succesful Authentication the OpenIDAuthenticationToken does not return the identity-url but an OpenIDUser object with the requested attributes.

@spring-issuemaster

Borut Bolčina said:

What is the status of this patch? Can it be applied against https://src.springframework.org/svn/spring-security/tags/spring-security-parent-2.0.4? If not, what is the url of the sources to be patched?

Thanks a lot!

@spring-issuemaster

Luke Taylor said:

I don’t know – it should be easy enough to check out the code and try the patch against it.

@spring-issuemaster

Borut Bolčina said:

The patch does not work against https://src.springframework.org/svn/spring-security/tags/spring-security-parent-2.0.4. A new one should be created for 2.0.4 release tag.

I see this issue is schedulled for 2.5.0 which is like a year away, so the patch for 2.0.4 is very welcome. Also a http://jira.springframework.org/browse/SEC-931 is schedulled for 2.5.0.

Does anybody have by any chance a patch for 2.0.4?

@spring-issuemaster

Christopher Schuster said:

I uploaded a new patch called “attribute-exchange_2.0.4.patch”. It’s a diff against spring-security-parent-2.0.4. The patch only effects the openid-subfolder.

I would appreciate if Spring Security 2.5 gets support for attribute-exchange because one of my Grails projects depends on it.

@spring-issuemaster

Luke Taylor said:

Thanks for the patch (again :-) ). The code has already changed a lot since 2.0.4, but I’ll try to get it in for the M1 release. It’d be very useful if you could do some testing at that point.

@spring-issuemaster

Luke Taylor said:

There is still not stable release of OpenID4Java available which will support this patch – 0.9.5 seems to have been under development for over a year now. So this wont be in M1.

@spring-issuemaster

Andreas Motl said:

Hi there,

i somehow took over from Matthias and Christopher and updated the patch to fix a possible race condition and some other minor issues.

We also released the patch to the Grails AcegiSecurity Plugin, which depends upon the modified Spring Security 2.0.4 library, see GRAILSPLUGINS-948.

Latest patches, an easy tutorial to get started and more information can now be found at http://myhpi.de/~andreas.motl/openid4grails/

Please also take a look at the discussion about the release of OpenID4Java 0.9.5.

Note: The updated patches don’t longer change any behavior related to SEC-931, instead using GrailsOpenIdAuthenticationProvider and GrailsOpenIdAuthenticationToken from org.codehaus.groovy.grails.plugins.springsecurity.openid.

Cheers,
Andreas.

@spring-issuemaster

Brian Brooks said:

Openid4java released 0.9.5 in June 2009.

@spring-issuemaster

Luke Taylor said:

Funny that. I just upgraded the version earlier today :).

@spring-issuemaster

Luke Taylor said:

I've added support for attribute exchange (sreg support will probably not be implemented, as it is effectively deprecated in favour of attribute exchange). I didn't follow the patches as there appeared to be issues with maintaining state within the consumer which would have introduced very nasty bugs. I've also left out any modification of the principal stored in the final Authentication object to include the attributes. The prinicpal will be the UserDetails object and the attributes will be available directly from the OpenIDAuthenticationToken as a List. If desired, users can override the createSuccessfulAuthentication method of OpenIDAuthenticationProvider to merge the returned attributes into a custom UserDetails.

I've also provided support in the namespace, so you can specify the attributes that should be added to the FetchRequest thus:


...





@spring-issuemaster

Andreas Motl said:

Great! I will try to find some time to investigate how this can be integrated with our attempt to integrate Attribute Exchange with Grails. Are you interested in this and willing to help out, if we are getting problems?

With kind regards,
Andreas.

P.S.: The issues with maintaining state within the consumer had already been resolved within the revised patches. I stored them inside the users session. How did you do it?

@spring-issuemaster

Luke Taylor said:

There didn't seem to be any need for storing the attribute state in the consumer, as the patches here were doing. I've implemented it so that the consumer maintains an immutable list of attributes built from the above configuration, in order to know how to build its FetchRequest. When the attributes are retrieved, it builds a separate list to store them in the authentication token. I tested it with my identity on myopenid.com and it worked OK.

@spring-issuemaster

Luke Taylor said:

Closing for now. Please raise any problems you encounter as separate issues.

@spring-issuemaster

Srikanth Pagadala said:

Hi Andreas

Did you get a chance to update the SpringSecurity plugin for Grails with this new enhancement, such auto-registration works out-of-the-box? (as you tried in your patched grails plugin)

Also would it be possible to enhance plugins with features from oAuth plugin + openId plugin?

that would really make the SpringSecurity plugin COMPLETE.

Thanks

@spring-issuemaster spring-issuemaster added this to the 3.0.0 M2 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment