-
Notifications
You must be signed in to change notification settings - Fork 12
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
improve saml config options #379
Conversation
we can set on company email domain which will be tested when authenticating via saml and if user email matches it it will assign the company to user. there is also new config for custom client saml configs which allows us to configure user specific login page which picks the config to be used for SAML auth. then it uses the company domain to match the config value. STTNHUB-222
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.
Looks fine, only a few small suggestions
@@ -81,6 +81,14 @@ export function EditCompanyDetails({company, companyTypes, users, errors, onChan | |||
error={errors ? errors.contact_email : null} | |||
/> | |||
|
|||
<TextInput |
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.
Should we hide this input if SSO is not enabled, as there would be no need to show an input that has no affect.
newsroom/auth/views.py
Outdated
@@ -255,3 +255,11 @@ def set_locale(): | |||
if locale and locale in app.config["LANGUAGES"]: | |||
flask.session["locale"] = locale | |||
return flask.redirect(flask.url_for("auth.login")) | |||
|
|||
|
|||
@blueprint.route("/login/<client>", methods=["GET"]) |
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.
Would this be better off in newsroom.auth.saml
file instead, so we're not adding an endpoint that has no use if SSO is not enabled?
<div class="card-body pt-1"> | ||
<form class="form" role="form" method="post" action="{{ url_for('auth.set_locale') }}"> | ||
<div class="form-group"> | ||
<label for="language">{{ gettext('Language') }}</label> |
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.
id
attribute matching this for
is missing on the select
tag
* improve saml config options we can set on company email domain which will be tested when authenticating via saml and if user email matches it it will assign the company to user. there is also new config for custom client saml configs which allows us to configure user specific login page which picks the config to be used for SAML auth. then it uses the company domain to match the config value. STTNHUB-222
we can set on company email domain which will be tested when authenticating via saml and if user email matches it it will assign the company to user.
there is also new config for custom client saml configs which allows us to configure user specific login page which picks the config to be used for SAML auth. then it uses the company domain to match the config value.
STTNHUB-222