Merge pull request #46 from status-im/develop

Progressive Github permissioning & client-side oauth tokens

project.clj

@@ -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"]

resources/migrations/20170322212119-do-not-store-oauth-token-in-db.up.sql

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

resources/sql/queries.sql

@@ -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

resources/templates/home.html

@@ -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" %}

src/clj/commiteth/db/users.clj

@@ -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]

src/clj/commiteth/github/core.clj

@@ -1,9 +1,11 @@
 (ns commiteth.github.core
-  (:require [tentacles.repos :as repos]
+  (:require [tentacles
-            [tentacles.users :as users]
+             [core :as tentacles]
-            [tentacles.repos :as repos]
+             [repos :as repos]
-            [tentacles.issues :as issues]
+             [oauth :as oauth]
-            [tentacles.core :as tentacles]
+             [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)))
 

src/clj/commiteth/layout.clj

@@ -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*]]
+            [ring.middleware.anti-forgery :refer [*anti-forgery-token*]]))
-            [commiteth.github.core :as github]))
 
 (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"))

src/clj/commiteth/routes/home.clj

@@ -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}]
+(defn home-page [{user-id :id
-  (layout/render "home.html" {:userId user-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}
+  (GET "/" {{user :identity} :session}
-       (home-page identity))
+       (home-page user))
   (GET "/logout" {session :session}
        (-> (redirect "/")
            (assoc :session (dissoc session :identity)))))

src/clj/commiteth/routes/redirect.clj

@@ -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})))))))

src/clj/commiteth/routes/services.clj

@@ -61,12 +61,12 @@
 
 (defn handle-toggle-repo [user params]
   (log/debug "handle-toggle-repo" user params)
-  (let [{token   :token
+  (let [{user-id :id} user
-           user-id :id} user
+        {repo-id :id
-          {repo-id :id
+         full-repo :full_name
-           full-repo :full_name
+         owner-avatar-url :owner-avatar-url
-           owner-avatar-url :owner-avatar-url
+         token   :token
-           repo    :name} params
+         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

src/cljs/commiteth/core.cljs

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

src/cljs/commiteth/db.cljs

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

src/cljs/commiteth/handlers.cljs

@@ -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
+(reg-event-fx
-  :initialize-db
+ :initialize-db
-  (fn [_ _]
+ [(inject-cofx :store)]
-    db/default-db)))
+ (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]]

src/cljs/commiteth/repos.cljs

@@ -54,8 +54,8 @@
                  (into [:div.ui.cards]
                        (map repo-card group-repos))]))))))
 
-
+(defn repos-page-token-ok []
-(defn repos-page []
+  (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]))))

src/cljs/commiteth/subscriptions.cljs

@@ -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]]