Skip to content

✨ Hide password settings when AUTH_TYPE indicates SSO#29

Merged
UsamaSadiq merged 7 commits into
foss-sandboxfrom
jawad/hide-password-tab
May 22, 2026
Merged

✨ Hide password settings when AUTH_TYPE indicates SSO#29
UsamaSadiq merged 7 commits into
foss-sandboxfrom
jawad/hide-password-tab

Conversation

@jawad-khan

Copy link
Copy Markdown

Summary

  • Plumb AUTH_TYPE from the frontend container environment into config.js as penpotAuthType (via the existing nginx entrypoint substitution pattern alongside flags / mPass sign-out).
  • Add cf/auth-type-sso?, treating the value case-insensitively as sso.
  • Hide the Account → Password sidebar entry when SSO is indicated.
  • Visiting #/settings/password redirects to #/settings/profile instead of rendering the password form.

Test plan

  • Frontend image / compose: set AUTH_TYPE=SSO (or uncomment the compose example). Reload app: Password is absent from settings sidebar.
  • Navigate directly to #/settings/password: URL flows to #/settings/profile; password form never stays visible.
  • With AUTH_TYPE unset (or not sso): Password still appears and the change-password UI works as before.
  • Confirm config.js served by the frontend includes var penpotAuthType = "sso" (or your value) after container start.

Deploy notes

  • Requires rebuilding/restarting the frontend image so nginx-entrypoint.sh runs against config.js.

Checklist

  • Choose the correct target branch; use develop by default.
  • Provide a brief summary of the changes introduced.
  • Add a detailed explanation of how to reproduce the issue and/or verify the fix, if applicable.
  • Include screenshots or videos, if applicable.
  • Add or modify existing integration tests in case of bugs or new features, if applicable.
  • Refactor any modified SCSS files following the refactor guide.
  • Check CI passes successfully.
  • Update the CHANGES.md file, referencing the related GitHub issue, if applicable.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR threads a deploy-time AUTH_TYPE environment variable into the frontend runtime config (config.jsapp.config) and uses it to hide the Account → Password UI when SSO is enabled, including redirecting direct visits to #/settings/password back to profile.

Changes:

  • Add penpotAuthType injection via the frontend nginx entrypoint and expose it through cf/auth-type-sso?.
  • Hide the Password entry in the settings sidebar when AUTH_TYPE indicates SSO.
  • Redirect /settings/password to /settings/profile and prevent the password form from rendering under SSO.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontend/src/app/main/ui/settings/sidebar.cljs Conditionally hides the Password settings nav entry when auth-type-sso? is true.
frontend/src/app/main/ui/settings.cljs Adds redirect logic and blocks password page rendering under SSO.
frontend/src/app/config.cljs Introduces auth-type-sso? helper reading globalThis.penpotAuthType.
docker/images/files/nginx-entrypoint.sh Injects AUTH_TYPE into config.js at container start.
docker/images/files/config.js Adds commented penpotAuthType placeholder for entrypoint substitution.
docker/images/docker-compose.yaml Documents AUTH_TYPE usage for the frontend service (commented example).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docker/images/files/nginx-entrypoint.sh Outdated
Comment on lines +37 to +41
# AUTH_TYPE (e.g. SSO): consumed by frontend to hide native password UX.
update_auth_type() {
if [ -n "$AUTH_TYPE" ]; then
echo "$(sed \
-e "s|^//var penpotAuthType = .*;|var penpotAuthType = \"$AUTH_TYPE\";|g" \
Comment thread frontend/src/app/main/ui/settings.cljs Outdated
(mf/with-effect [section]
(when (and (= section :settings-password)
(cf/auth-type-sso?))
(st/emit! (rt/nav :settings-profile))))

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread docker/images/files/nginx-entrypoint.sh Outdated
Comment on lines +40 to +44
# Escape `\`, `"`, and `&` so the generated JS literal and sed replacement stay valid.
update_auth_type() {
if [ -n "${AUTH_TYPE:-}" ]; then
local auth_esc
auth_esc=$(printf '%s' "$AUTH_TYPE" | sed -e 's/\\/\\\\/g' -e 's/&/\\\&/g' -e 's/"/\\"/g')
Comment on lines +41 to +48
update_auth_type() {
if [ -n "${AUTH_TYPE:-}" ]; then
local auth_esc
auth_esc=$(printf '%s' "$AUTH_TYPE" | sed -e 's/\\/\\\\/g' -e 's/&/\\\&/g' -e 's/"/\\"/g')
echo "$(sed \
-e "s#^//var penpotAuthType = .*;#var penpotAuthType = \"${auth_esc}\";#g" \
"$1")" > "$1"
fi

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

(st/emit! (rt/assign-exception {:type :authentication}))))
(st/emit! (rt/assign-exception {:type :authentication})))
(when (and (= section :settings-password)
(cf/auth-type-sso?))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot is this resolved now or not?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was resolved. The redirect is now guarded with (some? profile) (line 50), so it only fires after the profile is loaded. While profile is nil and SSO is active, the :settings-password case renders a loading spinner instead (lines 76-79), preventing any race between the auth exception and the redirect.

Comment thread docker/images/files/nginx-entrypoint.sh Outdated
Comment on lines +37 to +51
# AUTH_TYPE (e.g. SSO): consumed by frontend to hide native password UX.
#
# `#` separates pattern/replacement because values may contain "/" or "|".
# Escape `\`, `#`, `"`, and `&` so the generated JS literal and sed replacement stay valid.
update_auth_type() {
if [ -n "${AUTH_TYPE:-}" ]; then
local auth_esc
auth_esc=$(printf '%s' "$AUTH_TYPE" | sed \
-e 's/\\/\\\\/g' \
-e 's/#/\\#/g' \
-e 's/&/\\\&/g' \
-e 's/"/\\"/g')
echo "$(sed \
-e "s#^//var penpotAuthType = .*;#var penpotAuthType = \"${auth_esc}\";#g" \
"$1")" > "$1"
Comment on lines 71 to +75
:settings-password
[:& password-page]
(if (cf/auth-type-sso?)
;; Prefer profile UX over a loader until `rt/nav` updates the fragment.
[:& profile-page]
[:& password-page])
@jawad-khan jawad-khan force-pushed the jawad/hide-password-tab branch from 7e2540e to 4c11066 Compare May 20, 2026 12:53
@jawad-khan jawad-khan requested a review from Copilot May 20, 2026 12:53
@jawad-khan

Copy link
Copy Markdown
Author

@copilot

Copilot AI commented May 20, 2026

Copy link
Copy Markdown

@copilot

Addressed the two remaining review issues in commit 5656a23:

  1. nginx-entrypoint.shAUTH_TYPE is now normalized before injection: control characters are stripped (tr -d '[:cntrl:]'), whitespace is trimmed, and the value is lowercased (tr '[:upper:]' '[:lower:]'). This means AUTH_TYPE=SSO now produces var penpotAuthType = "sso" in config.js, consistent with the test plan.

  2. settings.cljs — The SSO redirect now uses {::rt/replace true} (rt/nav :settings-profile {} {::rt/replace true}), matching the pattern used in the subscription page. This replaces the history entry instead of pushing a new one, eliminating the back-button redirect loop.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread docker/images/files/nginx-entrypoint.sh Outdated
Comment on lines +53 to +61
| tr '[:upper:]' '[:lower:]')
auth_esc=$(printf '%s' "$auth_norm" | sed \
-e 's/\\/\\\\/g' \
-e 's/#/\\#/g' \
-e 's/&/\\\&/g' \
-e 's/"/\\"/g')
tmp="$(mktemp)" || return 1
if ! sed \
-e "s#^//var penpotAuthType = .*;#var penpotAuthType = \"${auth_esc}\";#g" \
Comment thread docker/images/files/nginx-entrypoint.sh Outdated
Comment on lines +42 to +56
# Substitution uses '#' as the sed delimiter (values may include "/" or "|").
# Normalises AUTH_TYPE: strip control characters, trim whitespace, lowercase.
# Then escapes \, #, ", & before embedding into a JS double-quoted string.
# Writes via a temp file + mv so a failed/interrupted sed cannot truncate config.js.
update_auth_type() {
if [ -n "${AUTH_TYPE:-}" ]; then
local auth_norm auth_esc tmp
# Strip control chars, trim leading/trailing whitespace, convert to lowercase.
auth_norm=$(printf '%s' "$AUTH_TYPE" \
| tr -d '[:cntrl:]' \
| sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//' \
| tr '[:upper:]' '[:lower:]')
auth_esc=$(printf '%s' "$auth_norm" | sed \
-e 's/\\/\\\\/g' \
-e 's/#/\\#/g' \
@jawad-khan

Copy link
Copy Markdown
Author

@copilot

Copilot AI commented May 20, 2026

Copy link
Copy Markdown

@copilot

Addressed both issues in commit de9566c:

  1. Cross-filesystem mktemp — changed to mktemp -p "$(dirname "$1")" so the temp file is always on the same filesystem as config.js, making the final mv a guaranteed atomic rename.

  2. Escaping complexity / injection risk — replaced the entire multi-step escape pipeline with a strict whitelist: after lowercasing, auth_norm is validated against ^[a-z0-9_-]+$. Any value containing spaces, #, &, \, ", newlines, or other special characters is rejected and a warning is printed to stderr. Since auth_norm is guaranteed to contain only safe characters, it can be embedded directly into the sed replacement and JS string with zero escaping needed.

@jawad-khan jawad-khan changed the base branch from foss-main to foss-sandbox May 21, 2026 12:10
@UsamaSadiq UsamaSadiq merged commit 05673be into foss-sandbox May 22, 2026
5 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants