Hi Mark,
My suspicion is that there's no great reason anymore. I don't know
historically why the code was written this way. My speculation would be
that the idea might have been to have a single login form which
attempted authentication via several services. But, that is not really a
plausible real life scenario, as modern authentication services often
send you off to another location anyhow (e.g. Shibboleth).
That section of the AuthenticationManager, according to git blame, may
date back to 2005 when the idea of stackable auth was first introduced
into DSpace:
https://github.com/DSpace/DSpace/blame/master/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationManager.java#L63
So, rather than dwell too much on the reason why it was built this way,
it seems like we should investigate options to modernize our
Authentication system in general. Personally, I feel this is an area
where we really should just be using a third-party Authentication plugin
(as noted in DS-1566) rather than continually maintaining a 10 year old
custom Authentication class.
https://jira.duraspace.org/browse/DS-1566
Just my two cents here though! :)
- Tim
On 5/4/2015 2:23 PM, Mark H. Wood wrote:
Followup to IRC conversation with hpottinger.
Please remind me why we do this. If there are two stacked
AuthenticationMethods which happen to use the same identifiers, we
could ignore the user's choice and always authenticate using the first
one. At most one method is allowed to succeed before
AuthenticationManager.authenticate() returns, so the reason can't be
to let every method get a look at the login request.
Should we not rather have an AuthenticationMethod.authorize() in
addition to .authenticate()? A UI would tell AuthenticationManager
which method to use for authentication, and then AuthenticationManager
would call authorize() on every method. authenticate would *only*
verify credentials; authorize() would be for whatever a method would
like to do, such as decorating the session with additional
information, updating the EPerson or other records, etc. It might be
sensible to have authorize() take up the function of adding special
groups to the session.
--
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
___
Dspace-devel mailing list
Dspace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dspace-devel
--
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
___
Dspace-devel mailing list
Dspace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dspace-devel
