Conversation
Fixed checkstyle violations
Retrieve token from cookie only when checking an authenticated eperson
…d eperson" This reverts commit 10bbf80.
Add authorization cookie only in ShibbolethAuthenticationFilter
Fixed AuthenticationRestControllerIT
Fixed checkstyle violations
Invalidate authorization cookie on logout
add context commit after invalidating token
This reverts commit 64bc25f.
use authorization cookie only to check
fix use authorization cookie only to check
Invalidate authentication cookie once used
Add checck to user and password blank
fix check user and password blank
Revert check
Check session salt in shib auth
This reverts commit 462f20a.
add attemptAuthentication method to ShibbolethAuthenticationFilter
fix attemptAuthentication method on ShibbolethAuthenticationFilter
Added WWW-Authenticate header to authn/status response
Revert "use authorization cookie only to check"
replace Boolean type with primitive type
paulo-graca
left a comment
There was a problem hiding this comment.
As promised, I will try to summarize what we have done in order to have Logout within shibboleth and DSpace.
Basically, in terms of the Service Provider, we use the shibboleth's notify parameter to configure the IdP logout url:
https://wiki.shibboleth.net/confluence/display/SP3/Notify
<Notify Channel="front" Location="https://[MY-SERVER]/logout" />
When the logout event in DSpace is performed, then the Notify is triggered.
Some exchanges will occur between the SP and the IdP and an indication to IdP to clean the shibboleth authentication will also occur.
In our case we will have something like this: https://[MY-SERVER]/shibboleth.sso/Logout?notifying=1&index=1
To invoke the logout. We also clean local cookies too.
|
You won't need to expose the Shibboleth header on entire REST as you specified above:
If you want to limit what uses the Shibboleth headers, /server/api/authn/shibboleth and /server/api/authn/login should suffice |
Added getBaseUrl to Utils
Fixed default value of authentication-shibboleth.lazysession.loginurl
Replace ConfigurationManager with ConfigurationService
benbosman
left a comment
There was a problem hiding this comment.
Thanks @atarix83 for the changes
I've tested the REST part (without Angular), and it worked correctly
I've reviewed the last changes as well, and I had a few questions for this:
- If you'd be hosting on a different port number (not likely for PROD environments of course), I assume the
getBaseUrlfunction won't work correctly. Can you:- add the port number in the
getBaseUrlfunction - test the returned location is correct in AuthenticationRestControllerIT if
authentication-shibboleth.lazysession.loginurlis modified to contain a port number - test the returned location is correct in AuthenticationRestControllerIT if
authentication-shibboleth.lazysession.loginurlis modified to contain a full URL with hostname included)
- add the port number in the
- The
getShibURLfunction doesn't verify the behavior if the URL is set toShibboleth.sso/Login. Can you either adjust this in thegetShibURLfunction or add some comments in authentication-shibboleth.cfg explaining the allowed formats
Added port to getBaseUrl
Use configurationService to set default authentication-shibboleth.lazysession.loginurl in getShibURL method
Added methods to test shibboleth url using port
Added comments for authentication-shibboleth.lazysession.loginurl in authentication-shibboleth.cfg
|
@benbosman |
|
@benbosman Did you tested shibboleth authentication on a server after the upgrade to java 11 ? I've tried to install this PR but it doesn't work, indeed after been logged in with shibboleth on redirect to dspace I get a 403 forbidden. This is not related to this implementation because the same behaviour occurs also on the demo rest server. |
paulo-graca
left a comment
There was a problem hiding this comment.
Thank you @atarix83 for this important contribution! It's working as expected!
I Can see a
"WWW-Authenticate:
shibboleth realm="DSpace REST API",
,password realm="DSpace REST API"
|
I've just created: https://jira.lyrasis.org/browse/DS-4464 to address the need of a Central Authentication Service Logout solution. |
Fixed checkstyle violations
|
@atarix83 how is the https://dspace7.4science.cloud currently configured to work with Shibboleth:
I noticed I used to be able to use Shibboleth with the previous codebase: that was based on Apache 2.2, mod_ajp @paulo-graca which setup are you using the test the Shibboleth integration? |
benbosman
left a comment
There was a problem hiding this comment.
This works for me, I've also updated https://wiki.lyrasis.org/display/DSPACE/DSpace+7+Shibboleth+Configuration with my setup and the issue I encountered.
The only problem I still see is the warning I get in Chrome when using separate hostnames for REST and Angular:
A cookie associated with a cross-site resource at http://dspace7-rest.atmire.com/ was set without the
SameSiteattribute. A future release of Chrome will only deliver cookies with cross-site requests if they are set withSameSite=NoneandSecure. You can review cookies in developer tools under Application>Storage>Cookies and see more details at https://www.chromestatus.com/feature/5088147346030592 and https://www.chromestatus.com/feature/5633521622188032.
This can also be resolved in a follow-up PR
|
@benbosman |
|
@atarix83 I don't use mod_jk, but rather mod_ajp If that doesn't work, can you share your config similar to what Paulo did on the wiki? |
|
Looks good to me, and I see this is already at +2 from Paulo & Ben. Instructions for setup are drafted at https://wiki.lyrasis.org/display/DSPACE/DSpace+7+Shibboleth+Configuration |
This PR provide changes to make the shibboleth authentication work as discussed in DS-4396
A summary of what is done with this PR:
As discussed in the issue DS-4396 this implementation doesn't handle the
Access-Control-Allow-Originheader in the response to avoid CORS problem.To solve it, we use in the rest VH the following apache configuration :
this PR has been already merged in the dspace-cris 7 codebase so it is possible to see the behavior here: https://dspacecris7.4science.cloud/home