After spending some time to work on tests and resubmit the pull request,
I found that Misagh already refactored the code and included the
proposed fix. So I closed my 2nd pull request. The next 6.4.0 release
will finally fix this long standing bug. Thanks Misagh / Chris!
David
On 7/22/21 3:58 PM, Chris Durham wrote:
Same happened to me the first time - you do have to provide updated
Integration tests - in my case it turns out that the code was
technically broken in two places, such that existing integration tests
were incorrectly returning that it was ok when it wasn't, so fixing
that second issue meant that the existing integration tests were
already fit for purpose.
Whilst having to handle the integration tests may seem like a pain
when like you I thought I had a simple defect, having gone through the
discussion with Misagh I fully understand why it's important to ensure
they are correct as they may hide other issues as a result.
On Thursday, 22 July 2021 at 14:55:11 UTC-5 Chia-Ying Yang wrote:
I did submit a pull request
(https://github.com/apereo/cas/pull/5204
<https://github.com/apereo/cas/pull/5204>), but was promptly
rejected for not having associated tests. It was a really simple
fix IMHO. I'll do a little more research on how to add a test and
resubmit.
I would appreciate any and all help!
David
On 7/22/21 3:50 PM, Chris Durham wrote:
Hi,
We too have been bitten by this (and I assume it's a bug too).
If you can fix it and submit that as a pull-request to the main
cas project then it would make me very happy!
If you do submit the pull-request then I'm sure Misagh would be
able to point out if it was the wrong place - he has been very
helpful in guiding me through my first pull-request and getting
it to the point where he was happier with it.
(We had to do some nasty js fudging in our core theme to achieve
what we needed - but having themed /logout pages would be far
easier!)
Chris
On Wednesday, 21 July 2021 at 13:40:58 UTC-5 Chia-Ying Yang wrote:
I want to confirm whether this is a bug or not, in the
current master
branch.
I configured a custom theme for a registered service. If I
override
casLoginView.html via overlay
(src/main/resources/templates/[theme]/login/casLoginView.html),
then the
custom login page template is being used to render the login
page. But
if I override casLogoutView.html via overlay
(src/main/resources/templates/[theme]/logout/casLogoutView.html),
that
custom logout page template is not being used. In fact the
one from the
default theme is always used. I can even override it via
overlay
(src/main/resources/templates/logout/casLogoutView.html).
Additional details:
- after logging out locally, the user is being redirected to
the cas
server /cas/logout?service=[service ID] for single logout,
i.e. the
service is being supplied.
- during the logout flow, RegisteredServiceThemeResolver is
being
called, but the service retrieved from the flow scope is
null. Only
LogoutRequest is in the flow scope, and inside it I do see the
registered service matching what was supplied. Because the theme
resolver cannot determine the service, the default theme is
used.
If it is indeed a bug, I think I can fix it in
LogoutViewSetupAction.doInternalExecute() by placing the
service into
the flow scope. I don't know enough about Spring web flow to
know
whether this is the right approach or the right place to fix
it. I
would appreciate any feedback or suggestions!
--
- Website: https://apereo.github.io/cas
- Gitter Chatroom: https://gitter.im/apereo/cas
- List Guidelines: https://goo.gl/1VRrw7
- Contributions: https://goo.gl/mh7qDG
---
You received this message because you are subscribed to the Google Groups "CAS Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to cas-user+unsubscr...@apereo.org.
To view this discussion on the web visit
https://groups.google.com/a/apereo.org/d/msgid/cas-user/bf69f63a-4858-1058-bfeb-9b70bf1da22b%40gmail.com.
