Progressive Github permissioning & client-side oauth tokens

* require only user:email oauth scope when signing up
* if user wants to set bounties on repos, request additional oauth
  scopes
* do not store github access tokens on server side and use client-side
  localStorage instead

Fixes: #35
Fixes: #40
This commit is contained in:
Teemu Patja 2017-03-22 22:20:52 +02:00
parent a6690859cc
commit 3f0ff567a8
No known key found for this signature in database
GPG Key ID: F5B7035E6580FD4C
16 changed files with 159 additions and 85 deletions

View File

@ -46,7 +46,8 @@
[org.clojure/tools.nrepl "0.2.12"]
[com.cemerick/piggieback "0.2.1"]
[jarohen/chime "0.2.0"]
[com.andrewmcveigh/cljs-time "0.4.0"]]
[com.andrewmcveigh/cljs-time "0.4.0"]
[akiroz.re-frame/storage "0.1.1"]]
:min-lein-version "2.0.0"
:source-paths ["src/clj" "src/cljc"]

View File

@ -0,0 +1 @@
ALTER TABLE "public"."users" DROP COLUMN "token";

View File

@ -3,14 +3,13 @@
-- :name create-user! :<! :1
-- :doc creates a new user record
INSERT INTO users
(id, login, name, email, avatar_url, token, address, created)
(id, login, name, email, avatar_url, address, created)
SELECT
:id,
:login,
:name,
:email,
:avatar_url,
:token,
:address,
:created
WHERE NOT exists(SELECT 1
@ -24,17 +23,9 @@ UPDATE users
SET login = :login,
name = :name,
email = :email,
token = :token,
address = :address
WHERE id = :id;
-- :name update-user-token! :<! :1
-- :doc updates user token and returns updated user
UPDATE users
SET token = :token
WHERE id = :id
RETURNING id, login, name, email, token, address, created;
-- :name update-user-address! :! :n
UPDATE users
SET address = :address

View File

@ -25,13 +25,15 @@
var context = "{{servlet-context}}";
var csrfToken = "{{csrf-token}}";
var authorizeUrl = "{{authorize-url}}";
var authorizeUrlAdmin = "{{authorize-url-admin}}";
var token = "{{token}}";
var userId = "{{userId}}";
var adminToken = "{{admin-token}}";
var userId = "{{user-id}}";
var user = "{{login}}";
if (user === "") {
user = null;
}
var commitethVersion = "{{commitethVersion}}";
var commitethVersion = "{{commiteth-version}}";
</script>
{% style "https://cdnjs.cloudflare.com/ajax/libs/semantic-ui/2.1.8/semantic.min.css" %}

View File

@ -5,14 +5,13 @@
(:import [java.util Date]))
(defn create-user
[user-id login name email avatar-url token]
[user-id login name email avatar-url]
(jdbc/with-db-connection [con-db *db*]
(db/create-user! con-db
{:id user-id
:login login
:name name
:email email
:token token
:avatar_url avatar-url
:address nil
:created (new Date)})))
@ -33,12 +32,6 @@
(jdbc/with-db-connection [con-db *db*]
(db/update-user-address! con-db {:id user-id :address address})))
(defn update-user-token
"Updates user token and returns updated user"
[user-id token]
(jdbc/with-db-connection [con-db *db*]
(db/update-user-token! con-db {:id user-id :token token})))
(defn get-repo-owner
"Gets repository owner by given repository id"
[repo-id]

View File

@ -1,9 +1,11 @@
(ns commiteth.github.core
(:require [tentacles.repos :as repos]
[tentacles.users :as users]
[tentacles.repos :as repos]
[tentacles.issues :as issues]
[tentacles.core :as tentacles]
(:require [tentacles
[core :as tentacles]
[repos :as repos]
[oauth :as oauth]
[users :as users]
[repos :as repos]
[issues :as issues]]
[ring.util.codec :as codec]
[commiteth.config :refer [env]]
[clj-http.client :as http]
@ -24,14 +26,23 @@
(defn self [] (:github-user env))
(defn self-password [] (:github-password env))
(defn authorize-url []
(defn authorize-url [scope]
(let [params (codec/form-encode {:client_id (client-id)
:redirect_uri (redirect-uri)
:scope "admin:repo_hook repo user:email admin:org_hook"
:scope scope
:allow_signup true
:state (str (UUID/randomUUID))})]
(str "https://github.com/login/oauth/authorize" "?" params)))
(defn signup-authorize-url []
(authorize-url "user:email"))
(defn admin-authorize-url []
(authorize-url "admin:repo_hook repo user:email admin:org_hook"))
(defn access-settings-url []
(str "https://github.com/settings/connections/applications/" (client-id)))
(defn post-for-token
[code state]
(http/post "https://github.com/login/oauth/access_token"
@ -75,10 +86,10 @@
(map #(merge
{:owner-login (get-in % [:owner :login])}
{:owner-type (get-in % [:owner :type])}
{:owner-avatar-url (get-in % [:owner :avatar_url])}
(select-keys % repo-fields))
(repos/repos (merge (auth-params token) {:type "all"
:all-pages true})))
(filter #(not (:fork %)))
(filter #(-> % :permissions :admin)))]
(group-by :owner-login all-repos-with-admin-access)))

View File

@ -4,8 +4,7 @@
[markdown.core :refer [md-to-html-string]]
[ring.util.http-response :refer [content-type ok]]
[ring.util.anti-forgery :refer [anti-forgery-field]]
[ring.middleware.anti-forgery :refer [*anti-forgery-token*]]
[commiteth.github.core :as github]))
[ring.middleware.anti-forgery :refer [*anti-forgery-token*]]))
(parser/set-resource-path! (clojure.java.io/resource "templates"))
(parser/add-tag! :csrf-field (fn [_ _] (anti-forgery-field)))
@ -19,7 +18,6 @@
(parser/render-file
template
(assoc params
:authorize-url (github/authorize-url)
:page template
:csrf-token *anti-forgery-token*)))
"text/html; charset=utf-8"))

View File

@ -1,5 +1,6 @@
(ns commiteth.routes.home
(:require [commiteth.layout :as layout]
[commiteth.github.core :as github]
[compojure.core :refer [defroutes GET]]
[ring.util.response :refer [redirect]]
[ring.util.http-response :refer [ok header]]
@ -8,15 +9,21 @@
(defonce ^:const version (System/getProperty "commiteth.version"))
(defn home-page [{user-id :id login :login token :token}]
(layout/render "home.html" {:userId user-id
(defn home-page [{user-id :id
login :login
token :token
admin-token :admin-token}]
(layout/render "home.html" {:user-id user-id
:login login
:token token
:commitethVersion version}))
:admin-token admin-token
:authorize-url (github/signup-authorize-url)
:authorize-url-admin (github/admin-authorize-url)
:commiteth-version version}))
(defroutes home-routes
(GET "/" {{identity :identity} :session}
(home-page identity))
(GET "/" {{user :identity} :session}
(home-page user))
(GET "/logout" {session :session}
(-> (redirect "/")
(assoc :session (dissoc session :identity)))))

View File

@ -9,7 +9,8 @@
[ring.util.response :as response]
[commiteth.layout :refer [render]]
[cheshire.core :refer [generate-string]]
[clojure.tools.logging :as log]))
[clojure.tools.logging :as log]
[clojure.string :as str]))
@ -19,7 +20,7 @@
user-id :id
avatar-url :avatar_url} user
email (github/get-user-email token)]
(users/create-user user-id login name email avatar-url token)))
(users/create-user user-id login name email avatar-url)))
(defn- get-or-create-user
[token]
@ -28,17 +29,22 @@
user-id :id} user]
(log/debug "get-or-create-user" user)
(or
(users/update-user-token user-id token)
(users/get-user user-id)
(create-user token user))))
(defroutes redirect-routes
(GET "/callback" [code state]
(let [resp (github/post-for-token code state)
body (keywordize-keys (codec/form-decode (:body resp)))
scope (:scope body)
access-token (:access_token body)]
(log/debug "github sign-in callback, response body:" body)
(if (:error body)
;; Why does Mist browser sends two redirects at the same time? The latter results in 401 error.
(response/redirect "/")
(let [user (get-or-create-user access-token)]
(let [admin-token? (str/includes? scope "repo")
token-key (if admin-token? :admin-token :token)
user (-> (get-or-create-user access-token)
(assoc token-key access-token))]
(-> (response/redirect "/")
(assoc :session {:identity user})))))))

View File

@ -61,12 +61,12 @@
(defn handle-toggle-repo [user params]
(log/debug "handle-toggle-repo" user params)
(let [{token :token
user-id :id} user
{repo-id :id
full-repo :full_name
owner-avatar-url :owner-avatar-url
repo :name} params
(let [{user-id :id} user
{repo-id :id
full-repo :full_name
owner-avatar-url :owner-avatar-url
token :token
repo :name} params
[owner _] (str/split full-repo #"/")
db-user (users/get-user (:id user))]
(if (empty? (:address db-user))
@ -90,9 +90,9 @@
(defn in? [coll elem]
(some #(= elem %) coll))
(defn handle-get-user-repos [user]
(defn handle-get-user-repos [user token]
(log/debug "handle-get-user-repos")
(let [github-repos (github/get-user-repos (:token user))
(let [github-repos (github/get-user-repos token)
enabled-repos (vec (repositories/get-enabled (:id user)))
repo-enabled? (fn [repo] (in? enabled-repos (:id repo)))
update-enabled (fn [repo] (assoc repo :enabled (repo-enabled? repo)))]
@ -164,7 +164,6 @@
:current-user user
(ok {:user (dissoc
(users/get-user (:id user))
:token
:email)}))
(POST "/address" []
:auth-rules authenticated?
@ -179,10 +178,12 @@
(if (= 1 result)
(ok)
(internal-server-error)))))
(GET "/repositories" []
(GET "/repositories" {:keys [params]}
:auth-rules authenticated?
:current-user user
(ok {:repositories (handle-get-user-repos user)}))
(ok {:repositories (handle-get-user-repos
user
(:token params))}))
(GET "/enabled-repositories" []
:auth-rules authenticated?
:current-user user

View File

@ -173,18 +173,22 @@
(defn load-user []
(if-let [login js/user]
(when-not (= login @active-user)
(println "active user changed, loading user data")
(reset! active-user login)
(rf/dispatch [:set-active-user
{:login login
:id (js/parseInt js/userId)
:token js/token}]))
(if (= login @active-user)
(do
(println "refresh with active user, updating tokens")
(rf/dispatch [:update-tokens js/token js/adminToken]))
(do
(println "active user changed, loading user data")
(reset! active-user login)
(rf/dispatch [:set-active-user
{:login login
:id (js/parseInt js/userId)}
js/token
js/adminToken])))
(reset! active-user nil)))
(defn load-data []
(doall
(map rf/dispatch
(doall (map rf/dispatch
[[:load-open-bounties]
[:load-activity-feed]
[:load-top-hunters]]))

View File

@ -8,4 +8,6 @@
:open-bounties []
:owner-bounties {}
:top-hunters []
:activity-feed []})
:activity-feed []
:gh-token nil
:gh-admin-token nil})

View File

@ -1,8 +1,19 @@
(ns commiteth.handlers
(:require [commiteth.db :as db]
[re-frame.core :refer [dispatch reg-event-db reg-event-fx reg-fx]]
[re-frame.core :refer [dispatch
reg-event-db
reg-event-fx
reg-fx
inject-cofx]]
[ajax.core :refer [GET POST]]
[cuerdas.core :as str]))
[cuerdas.core :as str]
[akiroz.re-frame.storage
:as rf-storage
:refer [reg-co-fx!]]))
(rf-storage/reg-co-fx! :commiteth {:fx :store
:cofx :store})
(reg-fx
:http
@ -23,13 +34,14 @@
:redirect
(fn [{:keys [path]}]
(println "redirecting to" path)
(set! (.-pathname js/location) path))
(set! (.-pathname js/location) path)))
(reg-event-db
:initialize-db
(fn [_ _]
db/default-db)))
(reg-event-fx
:initialize-db
[(inject-cofx :store)]
(fn [{:keys [db store]} [_]]
{:db (merge db/default-db store)}))
(reg-event-db
:assoc-in
@ -55,11 +67,42 @@
(fn [db _]
(dissoc db :flash-message)))
(defn update-if-not-empty [m k val]
(conj m (when (not (empty? val)) [k val])))
(defn update-local-storage-tokens [ls token admin-token]
(into {}
(-> ls
(update-if-not-empty :gh-token token)
(update-if-not-empty :gh-admin-token admin-token))))
(reg-event-fx
:set-active-user
(fn [{:keys [db]} [_ user]]
(fn [{:keys [db store]} [_ user token admin-token]]
{:db (assoc db :user user)
:dispatch [:load-user-profile]}))
:dispatch-n [[:update-tokens token admin-token]
[:load-user-profile]]}))
(reg-event-fx
:update-tokens
[(inject-cofx :store)]
(fn [{:keys [db store]} [_ token admin-token]]
(let [ls-data (update-local-storage-tokens store token admin-token)]
(println "update-tokens, ls-data:" ls-data)
{:db (merge db ls-data)
:store ls-data
:dispatch [:load-user-repos]})))
;; copied from plumbing.core to avoid cljsbuild warnings
(defn dissoc-in
[m [k & ks]]
(when m
(if-let [res (and ks (dissoc-in (get m k) ks))]
(assoc m k res)
(let [res (dissoc m k)]
(when-not (empty? res)
res)))))
(reg-event-fx
:sign-out
@ -156,6 +199,7 @@
{:db (assoc db :repos-loading? true)
:http {:method GET
:url "/api/user/repositories"
:params {:token (:gh-admin-token db)}
:on-success #(dispatch [:set-user-repos (:repositories %)])
:on-error #(dispatch [:set-flash-message
:error "Failed to load repositories"])
@ -175,7 +219,8 @@
(reg-event-fx
:toggle-repo
(fn [{:keys [db]} [_ repo]]
[(inject-cofx :store)]
(fn [{:keys [db store]} [_ repo]]
{:db (assoc db :repos (update-repo-state
(:repos db)
(:full_name repo)
@ -186,7 +231,12 @@
:on-success #(dispatch [:repo-toggle-success %])
:on-error #(dispatch [:repo-toggle-error repo %])
:finally #(println "finally" %)
:params (select-keys repo [:id :owner :owner-avatar-url :full_name :name])}}))
:params (merge {:token (:gh-admin-token store)}
(select-keys repo [:id
:owner
:owner-avatar-url
:full_name
:name]))}}))
(reg-event-db
@ -288,16 +338,6 @@
:error
(str "Failed to send transaction" e)]]})))))
;; copied from plumbing.core to avoid cljsbuild warnings
(defn dissoc-in
[m [k & ks]]
(when m
(if-let [res (and ks (dissoc-in (get m k) ks))]
(assoc m k res)
(let [res (dissoc m k)]
(when-not (empty? res)
res)))))
(reg-event-fx
:payout-confirmed
(fn [{:keys [db]} [_ issue-id]]

View File

@ -54,8 +54,8 @@
(into [:div.ui.cards]
(map repo-card group-repos))]))))))
(defn repos-page []
(defn repos-page-token-ok []
(println "repos-token-ok")
(let [repos-loading? (rf/subscribe [:repos-loading?])]
(fn []
(if @repos-loading?
@ -63,3 +63,15 @@
[:div.ui.active.inverted.dimmer
[:div.ui.text.loader "Loading"]]]
[repos-list]))))
(defn repos-page []
(let [gh-admin-token (rf/subscribe [:gh-admin-token])]
(fn []
(println "gh-admin-token" @gh-admin-token)
(if (empty? @gh-admin-token)
[:div.ui.container
[:div.ui.warning.message
[:i.warning.icon]
"To set bounties for your repositories or for organizations' repositories where you have access, you need to grant Commit ETH the required permissions"]
[:a.ui.button.small {:href js/authorizeUrlAdmin} "Enable"]]
[repos-page-token-ok]))))

View File

@ -51,6 +51,11 @@
(fn [db _]
(:activity-feed db)))
(reg-sub
:gh-admin-token
(fn [db _]
(:gh-admin-token db)))
(reg-sub
:get-in
(fn [db [_ path]]