[MAGNOLIA-2313] JcrAuthorizationModule should not extend JCRAuthenticationModule Created: 11/Aug/08 Updated: 23/Jan/13 Resolved: 28/Jan/09 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | core |
| Affects Version/s: | 3.6.1 |
| Fix Version/s: | 4.0, 3.6.4, 3.6.5 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Fabrizio Giustina | Assignee: | Fabrizio Giustina |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
| Template: |
|
| Acceptance criteria: |
Empty
|
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
| Date of First Response: |
| Description |
|
... or at least it shouldn't call login() in the JCRAuthenticationModule. The problem is that JcrAuthorizationModule should be usable to handle jcr authorization (expand groups and roles to ACLs), regardless of the authentication method used. But if you try to supply a different Authorization module, at least if this module uses different callbacks from the JcrAuthenticationModule, the login() method in the JCRAuthenticationModule will never pass. |
| Comments |
| Comment by Fabrizio Giustina [ 11/Aug/08 ] |
|
done, now JCRAuthorizationModule extends AbstractLoginModule and not JCRAuthenticationModule and avoid any repeated authentication step at login(). (PS: I never noticed this problem, but it definitively pops up when you try to use an authentication module which doesn't support NameCallbackHandler) |
| Comment by Jan Haderka [ 08/Sep/08 ] |
|
Could you please attach jaas configuration in which you would be using non magnolia Authorization module with magnolia Authentication? Do you use it to it to authenticate users against Magnolia user list (with passwords also kept in Magnolia) but to get users groups, roles and acls from other database? Thx. |
| Comment by Fabrizio Giustina [ 12/Sep/08 ] |
|
> jaas configuration in which you would be using non magnolia Authorization module with magnolia Authentication well, actually is a custom authentication module used with the magnolia standard authorization one. this is a snippet from a sample jaas.config: magnolia { We implemented a magnolia CAS login module (will be open sourced as soon as I can find the time to cleanup and document it properly, ping here if someone could need it now), which does the authentication by connecting to a CAS server. The Authentication module returns a user with the list of groups and roles, which are expanded by the standard magnolia authorization module. The problem here was that the authorization module was also checking the authentication token, crashing because CAS doesn't use an username/password authentication tocken but a ticket (a simple TextInputCallback). After the change the authorization module expects a user already autenticated and only expands its acls. |
| Comment by Philipp Bracher [ 12/Sep/08 ] |
|
Thanks it makes a lot of sense. We are now ready to release 3.6.2 with some important bugfixes we want to ship as soon as possible. Unfortunately we have some dependencies to this class in other EE modules and testing it now would requires a lot of extra work. So we have to skip that change for 3.6.2 but will re-add it immediately in both the 3.6 branch and 3.7 trunk. I hope you understand that we can't risk to break other EE modules without double checking it first. |
| Comment by Philipp Bracher [ 12/Sep/08 ] |
|
This is the patch to be applied after the release |
| Comment by Fabrizio Giustina [ 12/Sep/08 ] |
|
> I hope you understand that we can't risk to break other EE modules without double checking it first. Can we schedule a 3.6.3 with this change before branching for 3.7 anyway? It's a pretty important fix and I would like to use an unpatched 3.6.x in production... |
| Comment by Magnolia International [ 18/Dec/08 ] |
|
Reapplied patch on trunk and 3.6 branch |
| Comment by Magnolia International [ 20/Jan/09 ] |
|
What's the status of this? Do we need any documentation / changelog, does this mean any potential configuration change for users ? |
| Comment by Jan Haderka [ 22/Jan/09 ] |
|
As far as I can tell, it means that anyone having custom module extending this will need to recompile, but nothing else. |
| Comment by Magnolia International [ 28/Jan/09 ] |
|
right then, patch applied. |