-
Notifications
You must be signed in to change notification settings - Fork 275
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
Fix/5476 #5479
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
aup_template
andlanding_template
fields are missing when editing an SAML portal module.
Not sure if it's expected. -
If I set
with_aup=0
on a SAML portal module, AUP is always displayed. Tested with
[saml_portal]
source_id=saml
actions=
fields_to_save=
custom_fields=
description=saml_portal
with_aup=0
signup_template=signin.html
pid_field=email
aup_template=aup_text.html
type=Authentication::SAML
Hello @julsemaan, could you provide me your feedback ? Thanks. |
I tested it and it worked fine on my end before your comment. I will have to revisit this and see where is the issue but it did work when I pushed the code I'll also check to add the missing fields in the admin |
Regarding issue with |
@nqb, the with_aup is properly honored on my end conf:
|
I will test again. |
It finally worked without any change on my side (except a reboot). I will test your new changes. |
<button id="button" type="submit" name="submit" class="c-btn c-btn--primary u-1/1 c-card--hidden"> | ||
[% i18n('Login') %] | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm missing something, there is no need for this code here.
This code add an invisible button below "I accept the terms", you can click on it and you got "You must accept the terms and conditions"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That "I accept the terms" needs a target to submit the form
You can try to remove it and we'll see if it still works but AFAIK this is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right.
Description
Adjustments for SAML + OAuth for the portal modules
Impacts
SAML + OAuth portal modules
Issue
fixes #5476
Delete branch after merge
YES
Checklist
(REQUIRED) - [yes, no or n/a]
NEWS file entries
Enhancements
Bug Fixes