github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-07-29 17:36:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix SSO review

Browse Source
This commit is contained in:
Marina López 2026-07-28 13:39:33 +02:00
parent 4cd34b22e2
commit 056cd3d379
19 changed files with 251 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
backend/resources/app/email/invite-to-org/en.html
View File

@ -219,9 +219,15 @@
<div
style="font-family:Source Sans Pro, sans-serif;font-size:16px;line-height:150%;text-align:left;color:#000000;">
"{{ organization.name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to its
teams and files goes
through your organization's identity provider. If you can't get in, your account probably isn't
in the directory yet.
teams and files now goes through your organization's identity provider.
</div>
</td>
</tr>
<tr>
<td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
<div
style="font-family:Source Sans Pro, sans-serif;font-size:16px;line-height:150%;text-align:left;color:#000000;">
If you can't get in, your account probably isn't in the directory yet.
To get access, contact the organization owner.
</div>
</td>

6
backend/resources/app/email/invite-to-org/en.txt
View File

@ -3,8 +3,10 @@ Hello!
{{invited-by|abbreviate:25}} has invited you to join the organization “{{ organization.name|abbreviate:50 }}”.
{% if organization.sso-active %}
"{{ organization.name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to its teams and files goes
through your organization's identity provider. If you can't get in, your account probably isn't in the directory yet. To get access, contact the organization owner.
"{{ organization.name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to its teams and files now goes
through your organization's identity provider.
If you can't get in, your account probably isn't in the directory yet. To get access, contact the organization owner.
{% endif %}
Accept invitation using this link:

15
backend/resources/app/email/invite-to-team/en.html
View File

@ -197,10 +197,15 @@
<div
style="font-family:Source Sans Pro, sans-serif;font-size:16px;line-height:150%;text-align:left;color:#000000;">
"{{ organization.name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to
its
teams and files goes
through your organization's identity provider. If you can't get in, your account probably isn't
in the directory yet.
its teams and files now goes through your organization's identity provider.
</div>
</td>
</tr>
<tr>
<td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
<div
style="font-family:Source Sans Pro, sans-serif;font-size:16px;line-height:150%;text-align:left;color:#000000;">
If you can't get in, your account probably isn't in the directory yet.
To get access, contact the organization owner.
</div>
</td>
@ -257,4 +262,4 @@
</div>
</body>
</html>
</html>

6
backend/resources/app/email/invite-to-team/en.txt
View File

@ -3,8 +3,10 @@ Hello!
{{invited-by|abbreviate:25}} has invited you to join the team "{{ team|abbreviate:50 }}"{% if organization %}, part of the organization "{{ organization.name|abbreviate:50 }}"{% endif %}.
{% if organization.sso-active %}
"{{ organization.name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to its teams and files goes
through your organization's identity provider. If you can't get in, your account probably isn't in the directory yet. To get access, contact the organization owner.
"{{ organization.name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to its teams and files now goes
through your organization's identity provider.
If you can't get in, your account probably isn't in the directory yet. To get access, contact the organization owner.
{% endif %}
Accept invitation using this link:

14
backend/resources/app/email/organization-setup-sso/en.html
View File

@ -179,7 +179,7 @@
<td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
<div
style="font-family:Source Sans Pro, sans-serif;font-size:16px;line-height:150%;text-align:left;color:#000000;">
Hi{% if user-name %} {{ user-name|abbreviate:25 }}{% endif %},
Hi,
</div>
</td>
</tr>
@ -188,8 +188,16 @@
<div
style="font-family:Source Sans Pro, sans-serif;font-size:16px;line-height:150%;text-align:left;color:#000000;">
"{{ organization-name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to its
teams and files goes through your organization's identity provider. If you can't get in, your
account probably isn't in the directory yet. To get access, contact the organization owner.
teams and files now goes through your organization's identity provider.
</div>
</td>
</tr>
<tr>
<td align="left" style="font-size:0px;padding:10px 25px;word-break:break-word;">
<div
style="font-family:Source Sans Pro, sans-serif;font-size:16px;line-height:150%;text-align:left;color:#000000;">
If you can't get in, your account probably isn't in the directory yet. To get access, contact the
organization owner.
</div>
</td>
</tr>

2
backend/resources/app/email/organization-setup-sso/en.subj
View File

@ -1 +1 @@
“{{ organization-name|abbreviate:25 }}” has set up single sign-on (SSO) in Penpot
“{{ organization-name|abbreviate:25 }}” uses single sign-on

7
backend/resources/app/email/organization-setup-sso/en.txt
View File

@ -1,7 +1,8 @@
Hello!
Hi,
"{{ organization-name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to its teams and files goes
through your organization's identity provider. If you can't get in, your account probably isn't in the directory yet. To get access, contact the organization owner.
"{{ organization-name|abbreviate:50 }}" has set up single sign-on (SSO) in Penpot. Access to its teams and files now goes
through your organization's identity provider.
If you can't get in, your account probably isn't in the directory yet. To get access, contact the organization owner.
The Penpot team.

17
backend/src/app/auth/oidc.clj
View File

@ -766,25 +766,22 @@
(when-not (str/blank? value) value))
(defn org-sso-discovery-uri
"Return the OIDC discovery URI from an org SSO config, preferring :issuer."
"Return the OIDC discovery URI from an org SSO config."
[sso]
(or (non-blank-uri (:issuer sso))
(non-blank-uri (:base-url sso))))
(non-blank-uri (:issuer sso)))
(defn prepare-org-sso-provider
"Build an OIDC provider map dynamically from the Nitrate org SSO config.
Uses OIDC discovery via :base-url (or :issuer as fallback) when
token/auth/user URIs are absent."
[cfg {:keys [client-id client-secret base-url issuer scopes]}]
Uses OIDC discovery via :issuer when token/auth/user URIs are absent."
[cfg {:keys [client-id client-secret issuer]}]
(prepare-oidc-provider cfg
{:type "oidc"
:client-id client-id
:client-secret client-secret
:base-uri (some-> (or (non-blank-uri base-url)
(non-blank-uri issuer))
:base-uri (some-> (non-blank-uri issuer)
(str/rtrim "/")
(str "/"))
:scopes (into default-oidc-scopes (or scopes #{}))
:scopes default-oidc-scopes
:skip-ssrf-check? true}))
(defn build-org-sso-auth-redirect-uri
@ -797,7 +794,7 @@
(when-not issuer
(ex/raise :type :validation
:code :invalid-sso-config
:hint "missing issuer or base-url"))
:hint "missing issuer"))
(let [oidc-provider (or provider (prepare-org-sso-provider cfg sso))
state-token (tokens/generate cfg {:iss "oidc"
:dest-url dest-url

1
backend/src/app/email.clj
View File

@ -455,7 +455,6 @@
(def ^:private schema:organization-setup-sso
[:map
[:user-name {:optional true} [:maybe ::sm/text]]
[:organization-name ::sm/text]])
(def organization-setup-sso

2
backend/src/app/nitrate.clj
View File

@ -561,7 +561,7 @@
(call cfg :get-org-sso-by-team {:team-id team-id}))]
(if-not (:active sso)
{:authorized true :sso sso}
(if (or (:issuer sso) (:base-url sso))
(if-not (str/blank? (:issuer sso))
(let [props (:props session)
sso-map (get props :sso {})
organization-id (:organization-id sso)

31
backend/src/app/rpc/commands/nitrate.clj
View File

@ -663,7 +663,7 @@
(sv/defmethod ::check-nitrate-sso
"Check if a user needs to login into the organization SSO.
Accepts either team-id (to look up the org via the team) or organization-id directly.
Returns {:authorized true} when SSO is not active.
Returns {:authorized true} when SSO is not active or the user cannot access the team.
Returns {:authorized false :redirect-uri <url>} when SSO is active;
the client must redirect there. The OIDC provider itself handles
re-authentication transparently if the user already has an active SSO session."
@ -671,17 +671,22 @@
::doc/added "2.19"
::sm/params schema:check-nitrate-sso
::nitrate/sso false}
[cfg {:keys [team-id organization-id url] :as params}]
[cfg {:keys [::rpc/profile-id team-id organization-id url] :as params}]
(if (contains? cf/flags :nitrate)
(let [request (rph/get-request params)
{:keys [authorized sso]} (nitrate/sso-session-authorized? cfg organization-id team-id request)]
(if authorized
{:authorized true}
(if (oidc/org-sso-discovery-uri sso)
{:authorized false
:redirect-uri (oidc/build-org-sso-auth-redirect-uri cfg sso
:dest-url url
:organization-id organization-id)}
{:authorized false
:redirect-uri nil})))
(if (and team-id
(not (teams/has-read-permissions? cfg profile-id team-id)))
;; Let the destination RPC enforce its own permissions. Starting SSO before
;; access is established sends unrelated users through the organization's IdP.
{:authorized true}
(let [request (rph/get-request params)
{:keys [authorized sso]} (nitrate/sso-session-authorized? cfg organization-id team-id request)]
(if authorized
{:authorized true}
(if (oidc/org-sso-discovery-uri sso)
{:authorized false
:redirect-uri (oidc/build-org-sso-auth-redirect-uri cfg sso
:dest-url url
:organization-id organization-id)}
{:authorized false
:redirect-uri nil}))))
{:authorized true}))

6
backend/src/app/rpc/management/nitrate.clj
View File

@ -946,14 +946,14 @@ RETURNING id, deleted_at;")
::sm/params [:map
[:organization-id ::sm/uuid]
[:updated-props ::sm/boolean]
[:became-active ::sm/boolean]]
[:announce-activation ::sm/boolean]]
::rpc/auth false}
[{:keys [::db/pool] :as cfg} {:keys [organization-id updated-props became-active]}]
[{:keys [::db/pool] :as cfg} {:keys [organization-id updated-props announce-activation]}]
(when updated-props
(rpc/invalidate-org-sso-cache-by-org! organization-id)
(session/clear-org-sso-sessions! pool organization-id))
(notifications/notify-organization-change-sso cfg organization-id)
(when became-active
(when announce-activation
(neh/send-organization-setup-sso-emails! cfg organization-id))
nil)

8
backend/src/app/rpc/nitrate/emails_helper.clj
View File

@ -23,7 +23,7 @@
AND deleted_at IS NULL")
(def ^:private sql:get-profiles-by-emails
"SELECT id, email, fullname, is_muted
"SELECT id, email, is_muted
FROM profile
WHERE email = ANY(?)
AND deleted_at IS NULL")
@ -37,7 +37,7 @@
(def ^:private xf:map-email (map :email))
(defn- recipients-by-emails
"Build `{:email :user-name :profile}` maps for a deduplicated email list."
"Build `{:email :profile}` maps for a deduplicated email list."
[conn emails]
(let [profiles (if (seq emails)
(let [emails-array (db/create-array conn "text" emails)]
@ -47,20 +47,18 @@
(map (fn [email]
(let [profile (get profile-by-email (str/lower email))]
{:email email
:user-name (:fullname profile)
:profile profile}))
emails)))
(defn- send-organization-setup-sso-email!
"Send the organization SSO setup email to a single recipient, when allowed."
[conn organization-name {:keys [email user-name profile]}]
[conn organization-name {:keys [email profile]}]
(when (or (nil? profile)
(eml/allow-send-emails? conn profile))
(eml/send! {::eml/conn conn
::eml/factory eml/organization-setup-sso
:public-uri (cf/get :public-uri)
:to email
:user-name user-name
:organization-name organization-name})))
(defn- get-org-sso-notify-recipients

9
backend/test/backend_tests/rpc_management_nitrate_test.clj
View File

@ -1588,7 +1588,7 @@
params {::th/type :notify-organization-sso-change
:organization-id org-id
:updated-props false
:became-active true}]
:announce-activation true}]
;; --- Setup: insert invitations ---
;; Member also has a pending invitation: should still receive only one email.
@ -1645,7 +1645,7 @@
params {::th/type :notify-organization-sso-change
:organization-id (uuid/random)
:updated-props false
:became-active false}]
:announce-activation false}]
(with-redefs [eml/send! (fn [params] (swap! sent conj params))]
(th/management-command! params))
(t/is (empty? @sent))))
@ -1658,7 +1658,7 @@
:organization-id org-id
:client-id "test-client"
:client-secret "test-secret"
:base-url "https://idp.example.com"}))]
:issuer "https://idp.example.com"}))]
(t/is (th/success? out))
(t/is (true? (-> out :result :valid)))))
@ -1671,7 +1671,7 @@
(t/is (th/success? out))
(t/is (false? (-> out :result :valid)))))
(t/deftest check-organization-sso-uses-issuer-when-base-url-is-blank
(t/deftest check-organization-sso-passes-issuer-to-validation
(let [org-id (uuid/random)
out (with-redefs [oidc/is-organization-sso-config-valid?
(fn [_cfg sso]
@ -1682,7 +1682,6 @@
:organization-id org-id
:client-id "test-client"
:client-secret "test-secret"
:base-url ""
:issuer "https://idp.example.com/"}))]
(t/is (th/success? out))
(t/is (true? (-> out :result :valid)))))

96
backend/test/backend_tests/rpc_nitrate_test.clj
View File

@ -6,6 +6,7 @@
(ns backend-tests.rpc-nitrate-test
(:require
[app.auth.oidc :as oidc]
[app.common.json :as json]
[app.common.time :as ct]
[app.common.uuid :as uuid]
@ -59,10 +60,105 @@
:get-org-summary org-summary
nil)))
(defn- active-sso-call-mock
[team-id organization-id organization-owner-id]
(fn [_cfg method params]
(case method
:get-team-org
(when (= team-id (:team-id params))
{:id team-id
:organization {:id organization-id
:owner-id organization-owner-id}})
:get-org-sso-by-team
{:active true
:issuer "https://idp.example.com"
:organization-id organization-id}
nil)))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Tests
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(t/deftest check-nitrate-sso-skips-gate-without-team-access
(let [team-owner (th/create-profile* 1 {:is-active true})
external-profile (th/create-profile* 2 {:is-active true})
team (th/create-team* 1 {:profile-id (:id team-owner)})
organization-id (uuid/random)
params (with-meta
{::th/type :check-nitrate-sso
::rpc/profile-id (:id external-profile)
:team-id (:id team)
:url "https://penpot.example.com/#/workspace"}
{::http/request {}})]
(binding [cf/flags (conj cf/flags :nitrate)]
(with-redefs [nitrate/call
(active-sso-call-mock
(:id team)
organization-id
(:id team-owner))
oidc/build-org-sso-auth-redirect-uri
(constantly "https://idp.example.com/authorize")]
(let [out (th/command! params)]
(t/is (th/success? out))
(t/is (= {:authorized true} (:result out))))))))
(t/deftest check-nitrate-sso-keeps-gate-for-team-member
(let [team-owner (th/create-profile* 1 {:is-active true})
team (th/create-team* 1 {:profile-id (:id team-owner)})
organization-id (uuid/random)
redirect-uri "https://idp.example.com/authorize"
params (with-meta
{::th/type :check-nitrate-sso
::rpc/profile-id (:id team-owner)
:team-id (:id team)
:url "https://penpot.example.com/#/workspace"}
{::http/request {}})]
(binding [cf/flags (conj cf/flags :nitrate)]
(with-redefs [nitrate/call
(active-sso-call-mock
(:id team)
organization-id
(:id team-owner))
oidc/build-org-sso-auth-redirect-uri
(constantly redirect-uri)]
(let [out (th/command! params)]
(t/is (th/success? out))
(t/is (= {:authorized false
:redirect-uri redirect-uri}
(:result out))))))))
(t/deftest check-nitrate-sso-keeps-gate-for-non-member-org-owner
(let [team-owner (th/create-profile* 1 {:is-active true})
org-owner (th/create-profile* 2 {:is-active true})
team (th/create-team* 1 {:profile-id (:id team-owner)})
organization-id (uuid/random)
redirect-uri "https://idp.example.com/authorize"
params (with-meta
{::th/type :check-nitrate-sso
::rpc/profile-id (:id org-owner)
:team-id (:id team)
:url "https://penpot.example.com/#/workspace"}
{::http/request {}})]
(binding [cf/flags (conj cf/flags :nitrate)]
(with-redefs [nitrate/organization-owner-of-team?
(fn [_cfg profile-id team-id]
(and (= (:id org-owner) profile-id)
(= (:id team) team-id)))
nitrate/call
(active-sso-call-mock
(:id team)
organization-id
(:id org-owner))
oidc/build-org-sso-auth-redirect-uri
(constantly redirect-uri)]
(let [out (th/command! params)]
(t/is (th/success? out))
(t/is (= {:authorized false
:redirect-uri redirect-uri}
(:result out))))))))
(t/deftest leave-org-happy-path-no-extra-teams
(let [profile-owner (th/create-profile* 1 {:is-active true})
profile-user (th/create-profile* 2 {:is-active true})

4
common/src/app/common/types/organization.cljc
View File

@ -70,7 +70,5 @@
[:active {:optional true} [:maybe :boolean]]
[:provider {:optional true} [:maybe :string]]
[:client-id {:optional true} [:maybe :string]]
[:base-url {:optional true} [:maybe :string]]
[:client-secret {:optional true} [:maybe :string]]
[:issuer {:optional true} [:maybe :string]]
[:scopes {:optional true} [:maybe [::sm/set ::sm/text]]]])
[:issuer {:optional true} [:maybe :string]]])

38
frontend/src/app/main/data/dashboard.cljs
View File

@ -687,20 +687,35 @@
(rx/of (dcm/change-team-role params)
(modal/hide)))))
(defn- check-team-sso
[team-id]
(let [url (rt/get-current-href)]
(->> (rp/cmd! :check-nitrate-sso {:team-id team-id :url url})
(rx/mapcat (fn [{:keys [authorized redirect-uri]}]
(if authorized
(rx/empty)
(if redirect-uri
(rx/of (rt/nav-raw :uri (str redirect-uri)))
(rx/empty))))))))
(defn handle-change-team-org
[{:keys [team notification]}]
(ptk/reify ::handle-change-team-org
ptk/WatchEvent
(watch [_ state _]
(let [current-team-id (:current-team-id state)
organization (:organization team)]
organization (:organization team)
current-team? (= (:id team) current-team-id)]
(when (and (contains? cf/flags :nitrate)
notification
(= (:id team) current-team-id))
(rx/of (ntf/show {:content (tr notification (:name organization))
:type :toast
:level :info
:timeout nil})))))
current-team?)
(rx/concat
(when notification
(rx/of (ntf/show {:content (tr notification (:name organization))
:type :toast
:level :info
:timeout nil})))
(when (:id organization)
(check-team-sso current-team-id))))))
ptk/UpdateEvent
(update [_ state]
(if (contains? cf/flags :nitrate)
@ -767,14 +782,7 @@
team (dm/get-in state [:teams team-id])
org-id (dm/get-in team [:organization :id])]
(when (= organization-id org-id)
(let [url (rt/get-current-href)]
(->> (rp/cmd! :check-nitrate-sso {:team-id team-id :url url})
(rx/mapcat (fn [{:keys [authorized redirect-uri]}]
(if authorized
(rx/empty)
(if redirect-uri
(rx/of (rt/nav-raw :uri (str redirect-uri)))
(rx/empty)))))))))))))
(check-team-sso team-id)))))))
(defn- process-message
[{:keys [type] :as msg}]

50
frontend/test/frontend_tests/data/dashboard_test.cljs Normal file
View File

@ -0,0 +1,50 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS INC Sucursal en España SL
(ns frontend-tests.data.dashboard-test
(:require
[app.common.uuid :as uuid]
[app.config :as cf]
[app.main.data.dashboard :as dd]
[app.main.repo :as rp]
[app.main.router :as rt]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.mock :as mock]
[potok.v2.core :as ptk]))
(t/deftest moving-current-team-into-sso-organization-redirects
(t/async done
(let [team-id (uuid/next)
organization {:id (uuid/next) :name "OrgA"}
current-url (str "https://penpot.example.com/#/dashboard/recent?team-id=" team-id)
redirect-url "https://idp.example.com/authorize"
state {:current-team-id team-id}
event (dd/handle-change-team-org
{:team {:id team-id :organization organization}
:notification nil})]
(mock/with-mocks
{cf/flags (conj cf/flags :nitrate)
rp/cmd! (mock/stub
(fn [cmd params]
(if (= [:check-nitrate-sso
{:team-id team-id :url current-url}]
[cmd params])
(rx/of {:authorized false :redirect-uri redirect-url})
(rx/throw (ex-info "unexpected RPC" {:cmd cmd :params params})))))
rt/get-current-href (mock/stub (constantly current-url))}
(fn [done']
(->> (ptk/watch event state nil)
(rx/reduce conj [])
(rx/subs!
(fn [events]
(t/is (= [::rt/nav-raw] (mapv ptk/type events))))
(fn [error]
(t/is false (str "unexpected error: " error))
(done'))
(fn []
(done')))))
done))))

2
frontend/test/frontend_tests/runner.cljs
View File

@ -8,6 +8,7 @@
[frontend-tests.code-gen-style-test]
[frontend-tests.composable-tests.comp.sync-test]
[frontend-tests.copy-as-svg-test]
[frontend-tests.data.dashboard-test]
[frontend-tests.data.nitrate-test]
[frontend-tests.data.repo-test]
[frontend-tests.data.store-test]
@ -96,6 +97,7 @@
'frontend-tests.code-gen-style-test
'frontend-tests.composable-tests.comp.sync-test
'frontend-tests.copy-as-svg-test
'frontend-tests.data.dashboard-test
'frontend-tests.data.nitrate-test
'frontend-tests.data.repo-test