Skip to content

Commit 0c28981

Browse files
authored
Allow access to app through org's role (#1627)
1 parent d9530d5 commit 0c28981

File tree

10 files changed

+223
-30
lines changed

10 files changed

+223
-30
lines changed

server/src/instant/dash/routes.clj

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -102,37 +102,80 @@
102102
(assert-valid-member-role! nil)
103103
(assert-valid-member-role! 1))
104104

105+
(defn has-at-least-role? [least-privilege-role user-role]
106+
(assert (contains? member-roles least-privilege-role) "Expected valid least-privilege-role")
107+
(and user-role
108+
(contains? member-roles user-role)
109+
(<= (ucoll/index-of least-privilege-role member-role-hierarchy)
110+
(ucoll/index-of user-role member-role-hierarchy))))
111+
105112
(defn assert-least-privilege! [least-privilege-role user-role]
106113
(assert (contains? member-roles least-privilege-role) "Expected valid least-privilege-role")
107114
(ex/assert-valid!
108115
:user-role
109116
user-role
110-
(when-not
111-
(and user-role
112-
(contains? member-roles user-role))
113-
[{:message "This is not a valid role"
114-
:expected member-roles}]))
117+
(or (when-not user-role
118+
[{:message (format "User is missing role %s."
119+
(name least-privilege-role))}])
120+
(when-not (contains? member-roles user-role)
121+
[{:message "This is not a valid role"
122+
:expected member-roles}])))
115123
(ex/assert-permitted! :allowed-member-role? user-role
116-
(<= (ucoll/index-of least-privilege-role member-role-hierarchy)
117-
(ucoll/index-of user-role member-role-hierarchy))))
124+
(has-at-least-role? least-privilege-role user-role)))
118125

119-
(defn get-member-role [app-id user-id]
120-
(keyword (:member_role (instant-app-members/get-by-app-and-user {:app-id app-id :user-id user-id}))))
126+
(defn get-app-member-role [app user-id]
127+
(keyword (:member_role (instant-app-members/get-by-app-and-user {:app-id (:id app)
128+
:user-id user-id}))))
129+
130+
(defn get-org-member-role [app user-id]
131+
(when-let [org-id (:org_id app)]
132+
(keyword (:role (instant-org-members/get-by-org-and-user {:org-id org-id
133+
:user-id user-id})))))
121134

122135
(defn req->app-and-user!
123136
([req] (req->app-and-user! :owner req))
124137
([least-privilege req]
125138
(let [app-id (ex/get-param! req [:params :app_id] uuid-util/coerce)
126139
{app-creator-id :creator_id :as app} (app-model/get-by-id! {:id app-id})
127140
{user-id :id :as user} (req->auth-user! req)
128-
subscription (instant-subscription-model/get-by-app-id {:app-id app-id})]
129-
130-
(assert-least-privilege!
131-
least-privilege
132-
(cond
133-
(= user-id app-creator-id) :owner
134-
(stripe/pro-plan? subscription) (get-member-role app-id user-id)))
135-
{:app app :user user :subscription subscription})))
141+
app-subscription (instant-subscription-model/get-by-app-id {:app-id app-id})
142+
org-subscription (when-let [org-id (:org_id app)]
143+
(instant-subscription-model/get-by-org-id {:org-id org-id}))
144+
app-member-role (if (= user-id app-creator-id)
145+
:owner
146+
(get-app-member-role app user-id))
147+
good-app-role? (has-at-least-role? least-privilege app-member-role)
148+
org-member-role (get-org-member-role app user-id)
149+
good-org-role? (has-at-least-role? least-privilege org-member-role)]
150+
151+
(cond (or (and app-member-role
152+
good-app-role?
153+
(or (= :owner app-member-role)
154+
(stripe/plan-supports-members? app-subscription)
155+
(stripe/plan-supports-members? org-subscription)))
156+
157+
(and org-member-role
158+
good-org-role?
159+
(or (= :owner org-member-role)
160+
(stripe/plan-supports-members? org-subscription))))
161+
;; This is the only success case. The user has access through
162+
;; either the app or the org.
163+
{:app app :user user}
164+
165+
;; Has no role
166+
(and (not app-member-role)
167+
(not org-member-role))
168+
(ex/throw-validation-err! :user-role nil [{:message (format "User is missing role %s."
169+
(name least-privilege))}])
170+
171+
;; Has a role, but not one good enough to get access
172+
(and (not good-app-role?)
173+
(not good-org-role?))
174+
(ex/assert-permitted! :allowed-member-role? (or app-member-role org-member-role) false)
175+
176+
;; Has a role, but plan doesn't support members
177+
:else
178+
(ex/throw-insufficient-plan! {:capability "multiple members"})))))
136179

137180
(defn req->app-and-user-accepting-platform-tokens! [least-privilege scope req]
138181
(let [token (http-util/req->bearer-token! req)]
@@ -796,9 +839,9 @@
796839
(ex/throw-record-not-unique! :instant-subscription))
797840
{customer-id :id} (instant-stripe-customer-model/get-or-create-for-org! {:org org
798841
:user-email user-email})
799-
metadata (tool/inspect {"org-id" org-id
800-
"user-id" user-id
801-
"subscription-type-id" stripe/STARTUP_SUBSCRIPTION_TYPE})
842+
metadata {"org-id" org-id
843+
"user-id" user-id
844+
"subscription-type-id" stripe/STARTUP_SUBSCRIPTION_TYPE}
802845
description (str "Org name: " org-title)
803846
session-params {"success_url" (str (config/stripe-success-url) "&org=" org-id)
804847
"cancel_url" (str (config/stripe-cancel-url) "&org=" org-id)

server/src/instant/fixtures.clj

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@
1616
[instant.model.instant-user-refresh-token :as instant-user-refresh-token-model]
1717
[instant.model.member-invites :as member-invites]
1818
[instant.model.org :as org-model]
19+
[instant.model.org-members :as instant-org-members]
1920
[instant.model.rule :as rule-model]
20-
[instant.stripe :refer [PRO_SUBSCRIPTION_TYPE]]
21+
[instant.stripe :as stripe :refer [PRO_SUBSCRIPTION_TYPE]]
2122
[instant.db.pg-introspect :as pg-introspect]
2223
[instant.jdbc.sql :as sql]
2324
[instant.jdbc.aurora :as aurora]
@@ -273,3 +274,46 @@
273274
(app-model/delete-immediately-by-id! {:id app-id})
274275
(instant-app-members/delete-by-id! {:id (:id member)})
275276
(member-invites/delete-by-id! {:id (:id invite)})))))
277+
278+
(defn with-startup-org [f]
279+
(with-user
280+
(fn [owner]
281+
(with-user
282+
(fn [collaborator]
283+
(with-user
284+
(fn [admin]
285+
(with-user
286+
(fn [outside-user]
287+
(with-org
288+
(:id owner)
289+
(fn [org]
290+
(with-empty-app
291+
(fn [app]
292+
(let [stripe-customer (sql/execute-one! (aurora/conn-pool :write)
293+
["insert into instant_stripe_customers (id, org_id) values (?::text, ?::uuid) returning *"
294+
(str "test_" (crypt-util/random-hex 8))
295+
(:id org)])
296+
subscription (instant-subscription-model/create! {:user-id (:id owner)
297+
:org-id (:id org)
298+
:subscription-type-id stripe/STARTUP_SUBSCRIPTION_TYPE
299+
:stripe-customer-id (:id stripe-customer)
300+
:stripe-subscription-id (str "fake_sub_" (random-uuid))
301+
:stripe-event-id (str "fake_evt_" (random-uuid))})]
302+
(sql/do-execute! (aurora/conn-pool :write) ["update apps set org_id = ?::uuid where id = ?::uuid"
303+
(:id org)
304+
(:id app)])
305+
(sql/do-execute! (aurora/conn-pool :write) ["update orgs set subscription_id = ?::uuid where id = ?::uuid"
306+
(:id subscription)
307+
(:id org)])
308+
(instant-org-members/create! {:org-id (:id org)
309+
:user-id (:id collaborator)
310+
:role "collaborator"})
311+
(instant-org-members/create! {:org-id (:id org)
312+
:user-id (:id admin)
313+
:role "admin"}))
314+
(f {:app app
315+
:org org
316+
:owner owner
317+
:collaborator collaborator
318+
:admin admin
319+
:outside-user outside-user}))))))))))))))

server/src/instant/model/instant_subscription.clj

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
([conn {:keys [app-id]}]
6262
(sql/select-one ::get-by-app-id
6363
conn
64-
["SELECT s.id, s.app_id, s.stripe_subscription_id, t.name
64+
["SELECT s.id, s.app_id, s.stripe_subscription_id, t.name, s.subscription_type_id
6565
FROM instant_subscriptions s
6666
JOIN instant_subscription_types t on s.subscription_type_id = t.id
6767
WHERE s.app_id = ?::uuid
@@ -70,7 +70,11 @@
7070
app-id])))
7171

7272
(def get-by-org-id-q
73-
(uhsql/preformat {:select [:s.id :s.org_id :s.stripe_subscription_id, :t.name]
73+
(uhsql/preformat {:select [:s.id
74+
:s.org_id
75+
:s.stripe_subscription_id
76+
:t.name
77+
:s.subscription_type_id]
7478
:from :orgs
7579
:join [[:instant_subscriptions :s] [:= :s.id :orgs.subscription_id]
7680
[:instant_subscription_types :t] [:= :s.subscription_type_id :t.id]]

server/src/instant/model/org_members.clj

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,10 @@
2323
WHERE id = ?::uuid"
2424
role
2525
id])))
26+
27+
(defn get-by-org-and-user
28+
([params] (get-by-org-and-user (aurora/conn-pool :read) params))
29+
([conn {:keys [org-id user-id]}]
30+
(sql/select-one conn
31+
["SELECT * FROM org_members WHERE org_id = ?::uuid AND user_id = ?::uuid"
32+
org-id user-id])))

server/src/instant/oauth_apps/routes.clj

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
[hiccup2.core :as h]
55
[instant.auth.oauth :refer [verify-pkce!]]
66
[instant.config :as config]
7-
[instant.dash.routes :refer [get-member-role req->auth-user!]]
7+
[instant.dash.routes :refer [get-app-member-role get-org-member-role req->auth-user!]]
88
[instant.model.app :as app-model]
99
[instant.model.oauth-app :as oauth-app-model]
1010
[instant.runtime.routes :refer [format-cookie parse-cookie]]
@@ -230,8 +230,9 @@
230230
(= "localhost" (:host (uri/parse (:redirect_uri redirect)))))
231231
(not (app-model/get-by-id-and-creator {:app-id (:app_id oauth-app)
232232
:user-id (:id user)}))
233-
(not (get-member-role (:app_id oauth-app)
234-
(:id user))))
233+
(let [app (app-model/get-by-id! {:id (:app_id oauth-app)})]
234+
(not (or (get-app-member-role app (:id user))
235+
(get-org-member-role app (:id user))))))
235236
(oauth-app-model/deny-redirect! {:redirect-id redirect-id})
236237
(ex/throw+ {::ex/type ::ex/permission-denied
237238
::ex/message

server/src/instant/stripe.clj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@
2626
(defn pro-plan? [{:keys [name]}]
2727
(= name "Pro"))
2828

29+
(defn plan-supports-members? [{:keys [subscription_type_id]}]
30+
(or (= subscription_type_id PRO_SUBSCRIPTION_TYPE)
31+
(= subscription_type_id STARTUP_SUBSCRIPTION_TYPE)))
32+
2933
(defn ping-js-on-new-customer [{:keys [user-id org-id app-id]}]
3034
(let [{email :email} (instant-user-model/get-by-id {:id user-id})
3135
title (cond app-id

server/src/instant/util/exception.clj

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,11 @@
295295
:hint cause-message}))}
296296
e)))
297297

298+
(defn throw-insufficient-plan! [{:keys [capability]}]
299+
(throw+ {::type ::permission-denied
300+
::message (format "The plan for your app or organization does not support %s."
301+
capability)}))
302+
298303
;; -----------
299304
;; Validations
300305

server/test/instant/dash/routes_test.clj

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
[clj-http.client :as http]
44
[clojure.test :refer [deftest is testing]]
55
[instant.config :as config]
6-
[instant.fixtures :refer [random-email with-empty-app with-org with-user]]
6+
[instant.fixtures :refer [random-email with-empty-app with-org with-user with-startup-org]]
77
[instant.jdbc.aurora :as aurora]
88
[instant.jdbc.sql :as sql]
9-
[instant.util.json :refer [->json]]))
9+
[instant.util.json :refer [->json]]
10+
[instant.dash.routes :as route]))
1011

1112
(deftest app-invites-work
1213
(with-redefs [config/postmark-send-enabled? (constantly false)]
@@ -393,3 +394,87 @@
393394

394395
(is (not member))
395396
(is (= "revoked" (:status invite)))))))))))))
397+
398+
(deftest app-access-works-through-orgs
399+
(with-startup-org
400+
(fn [{:keys [app owner collaborator admin outside-user]}]
401+
;; Check a path available to all members of the app
402+
(let [auth-path (format "%s/dash/apps/%s/auth" config/server-origin (:id app))]
403+
(doseq [{:keys [user expected type]} [{:type "owner"
404+
:user owner
405+
:expected 200}
406+
{:type "collaborator"
407+
:user collaborator
408+
:expected 200}
409+
{:type "admin"
410+
:user admin
411+
:expected 200}
412+
{:type "outside-user"
413+
:user outside-user
414+
:expected 400}]]
415+
(testing type
416+
(is (= expected (:status (http/get auth-path
417+
{:throw-exceptions false
418+
:headers {:Authorization (str "Bearer " (:refresh-token user))
419+
:Content-Type "application/json"}
420+
:as :json})))))))
421+
422+
(testing "req->app-and-user!"
423+
(doseq [{:keys [user expected type role]} [{:type "owner"
424+
:user owner
425+
:role :owner
426+
:expected :ok}
427+
{:type "owner"
428+
:user owner
429+
:role :admin
430+
:expected :ok}
431+
{:type "owner"
432+
:user owner
433+
:role :collaborator
434+
:expected :ok}
435+
436+
{:type "collaborator"
437+
:user collaborator
438+
:role :owner
439+
:expected :error}
440+
{:type "collaborator"
441+
:user collaborator
442+
:role :admin
443+
:expected :error}
444+
{:type "collaborator"
445+
:user collaborator
446+
:role :collaborator
447+
:expected :ok}
448+
449+
{:type "admin"
450+
:user admin
451+
:role :owner
452+
:expected :error}
453+
{:type "admin"
454+
:user admin
455+
:role :admin
456+
:expected :ok}
457+
{:type "admin"
458+
:user admin
459+
:role :collaborator
460+
:expected :ok}
461+
462+
{:type "outside-user"
463+
:user outside-user
464+
:role :owner
465+
:expected :error}
466+
{:type "outside-user"
467+
:user outside-user
468+
:role :admin
469+
:expected :error}
470+
{:type "outside-user"
471+
:user outside-user
472+
:role :collaborator
473+
:expected :error}]]
474+
(testing (format "%s with role %s" type role)
475+
(let [req {:params {:app_id (:id app)}
476+
:headers {"authorization" (str "Bearer " (:refresh-token user))}}]
477+
(case expected
478+
:ok (is (= (:id app)
479+
(:id (:app (route/req->app-and-user! role req)))))
480+
:error (is (thrown? Exception (route/req->app-and-user! role req)))))))))))

server/test/instant/stripe_test.clj

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
(when (config/stripe-secret)
2222
(with-empty-app
2323
(fn [{app-id :id creator-id :creator_id}]
24-
(stripe/init)
2524
(let [customer (instant-stripe-customer-model/get-or-create-for-user! {:user {:id creator-id}})
2625
customer-id (:id customer)]
2726
(f (event-data {:app-id app-id
@@ -36,7 +35,6 @@
3635
(with-org
3736
(:id u)
3837
(fn [org]
39-
(stripe/init)
4038
(let [customer (instant-stripe-customer-model/get-or-create-for-org!
4139
{:org org
4240
:user-email (:email u)})

server/test/instant/test_core.clj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
[instant.core :as core]
99
[instant.config :as config]
1010
[instant.jdbc.aurora :as aurora]
11+
[instant.stripe :as stripe]
1112
[instant.system-catalog-migration :as system-catalog-migration]
1213
[instant.util.crypt :as crypt-util]
1314
[instant.util.tracer :as tracer]))
@@ -22,6 +23,7 @@
2223
(aurora/start)
2324
(core/start)
2425
(system-catalog-migration/ensure-attrs-on-system-catalog-app)
26+
(stripe/init)
2527
(let [results (test-suite-fn)]
2628
(aurora/stop)
2729
(core/stop)

0 commit comments

Comments
 (0)