Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP]: revert change that breaks ssoEnabled when pac4j-webflow configured #4332

Open
wants to merge 2 commits into
base: master
from

Conversation

@davidgelhar
Copy link
Contributor

commented Oct 8, 2019

This is a workaround to revert a change introduced in 6.1.0-RC4 in the following commit:

https://github.com/apereo/cas/blob/v6.1.0-RC4/support/cas-server-support-pac4j-webflow/src/main/java/org/apereo/cas/web/flow/DelegatedAuthenticationWebflowConfigurer.java#L60

that causes certain SAML logins to fail for services when delegated authentication (support-pac4j-webflow) is configured. The mere presence of the support-pac4j-webflow dependency causes the problem, even if no services are configured to use delegated auth.

Simply backing out the change is probably not the real solution (it was presumably there for a reason), but I don't understand the rationale for the change well enough to propose a better fix.

Procedure to reproduce:

  • include dependency cas-server-support-pac4j-webflow
  • configure a service with AccessStrategy "ssoEnabled" : false
  • do 2 SAML logins to that service

Result:
The second time, the browser shows a 500 error:

Error: Exception thrown in state 'viewLoginForm' of flow 'login'
...
ERROR [org.apache.catalina.core.ContainerBase.[Tomcat].[localhost].[/cas].[dispatcherServlet]] - <Servlet.service() for servlet [dispatcherServlet] in context with path [/cas] threw exception [Request processing failed; nested exception is java.lang.IllegalStateException: Exception occurred rendering view org.thymeleaf.spring5.view.ThymeleafView@393ad6eb] with root cause>
java.lang.IllegalStateException: Neither BindingResult nor plain target object for bean name 'credential' available as request attribute

@codecov

This comment has been minimized.

Copy link

commented Oct 14, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@49d1b05). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #4332   +/-   ##
=========================================
  Coverage          ?   39.39%           
  Complexity        ?     5138           
=========================================
  Files             ?     1667           
  Lines             ?    37012           
  Branches          ?     3416           
=========================================
  Hits              ?    14580           
  Misses            ?    21036           
  Partials          ?     1396

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49d1b05...89a6520. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.