[ 
https://issues.apache.org/jira/browse/OFBIZ-10047?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16292656#comment-16292656
 ] 

Jacques Le Roux commented on OFBIZ-10047:
-----------------------------------------

Hi James,

I'd really like to have this feature in the next to come R17 branch.  Here are 
some other points (apart OFBIZ-10057) I found reviewing and testing
{code}
+            } catch (ServletException e) {
+                e.printStackTrace();
{code}
This should use Debug.logError()

I also noticed this error in log, not sure what were the conditions so maybe a 
false issue.
{code}
2017-12-15 10:10:42,711 |jsse-nio-8443-exec-5 |RequestHandler                
|E| null
org.apache.ofbiz.webapp.event.EventHandlerException: Pre-Processor event 
[checkServletRequestRemoteUserLogin] did not return 'success'.
{code}
it's related with
{code}
-        if (allowRemoteUserLogin) {
+        if (useTomcatSSO || allowRemoteUserLogin) {
{code}
that I have to double check. BTW, though I see no real problem, I wonder about 
using the same code for external (remote) and internal (SSO) sign on.

Also minor, there a diamond is better:
{code}
        List<String> roles = new ArrayList<>();
        roles.add("PLACEHOLDER_ROLE");
{code}
PLACEHOLDER_ROLE should not be a problem since OFBiz uses its own roles 
handling.

So, apart OFBIZ-10057 that I have to double check, I find your patch ready to 
be committed, though I'd appreciate other reviews...

I also think that we can then deprecate the externalLoginKey stuff.

> Tomcat SSO
> ----------
>
>                 Key: OFBIZ-10047
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-10047
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>    Affects Versions: Trunk
>            Reporter: James Yong
>            Assignee: James Yong
>            Priority: Minor
>         Attachments: OFBIZ-10047.patch, OFBIZ-10047.patch
>
>
> Proposing Tomcat SSO to be used in OFBiz to improve on Single-Sign-On.
> This aim to fix the issues mentioned in OFBIZ-6963, OFBIZ-6994.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to