From fd54e61a49d534187f3b6bc1522c4d0dad32c8f1 Mon Sep 17 00:00:00 2001 From: flynn Date: Sat, 8 Jun 2019 23:51:04 +0200 Subject: [PATCH] better error handling for oauth restructure route handlers --- project.clj | 18 +++++----- src/clj/cat/handler.clj | 48 +++++++++++++++++---------- src/clj/cat/moauth.clj | 64 +++++++++++++++++++----------------- src/clj/cat/routes/admin.clj | 50 ++++++++++++++-------------- src/clj/cat/routes/oauth.clj | 11 +++---- 5 files changed, 100 insertions(+), 91 deletions(-) diff --git a/project.clj b/project.clj index 5723c9b..77f2515 100644 --- a/project.clj +++ b/project.clj @@ -1,17 +1,19 @@ (defproject cat "0.1.0-SNAPSHOT" :description "A cuddle graph for zeus people" - ;:url "http://example.com/FIXME" + ;:url "http://example.com/FIXME" :dependencies [[buddy "2.0.0"] - [com.cemerick/friend "0.2.3"] [cheshire "5.8.1"] [clj-oauth "1.5.5"] [clojure.java-time "0.3.2"] + [com.cemerick/friend "0.2.3"] [com.cognitect/transit-clj "0.8.313"] + [com.google.protobuf/protobuf-java "3.6.1"] [compojure "1.6.1"] [conman "0.8.3"] [cprop "0.1.13"] + [funcool/promesa "1.9.0"] [funcool/struct "1.3.0"] [luminus-immutant "0.2.4"] [luminus-migrations "0.6.3"] @@ -20,17 +22,16 @@ [markdown-clj "1.0.5"] [metosin/muuntaja "0.6.3"] [metosin/ring-http-response "0.9.1"] - [slingshot "0.12.1"] + [metosin/vega-tools "0.2.0"] [mount "0.1.15"] + [mysql/mysql-connector-java "8.0.12"] [nrepl "0.5.3"] [org.clojure/clojure "1.10.0"] [org.clojure/clojurescript "1.10.439" :scope "provided"] [org.clojure/tools.cli "0.4.1"] [org.clojure/tools.logging "0.4.1"] - ;[org.postgresql/postgresql "42.2.5"] - [mysql/mysql-connector-java "8.0.12"] - [com.google.protobuf/protobuf-java "3.6.1"] - ;https://www.webjars.org/ + ;[org.postgresql/postgresql "42.2.5"] + ;https://www.webjars.org/ [org.webjars.npm/bulma "0.7.2"] [org.webjars/font-awesome "5.6.1"] [org.webjars/webjars-locator "0.34"] @@ -38,8 +39,7 @@ [ring/ring-core "1.7.1"] [ring/ring-defaults "0.3.2"] [selmer "1.12.5"] - [metosin/vega-tools "0.2.0"] - [funcool/promesa "1.9.0"]] + [slingshot "0.12.1"]] diff --git a/src/clj/cat/handler.clj b/src/clj/cat/handler.clj index 3a5918f..c03c964 100644 --- a/src/clj/cat/handler.clj +++ b/src/clj/cat/handler.clj @@ -2,30 +2,42 @@ (:require [cat.middleware :as middleware] [cat.layout :refer [error-page]] [cat.routes.home :refer [home-routes]] - [cat.routes.oauth :refer [oauth-routes]] - [cat.routes.admin :refer [admin-routes]] - [compojure.core :refer [routes wrap-routes]] + [cat.routes.oauth :refer [oauth-init oauth-callback clear-session!]] + [cat.routes.admin :refer [set-admin! create-new-relation! create-user!]] + [compojure.core :refer [routes defroutes GET POST wrap-routes]] [ring.util.http-response :as response] [compojure.route :as route] [cat.env :refer [defaults]] + [clojure.tools.logging :as log] [mount.core :as mount])) (mount/defstate init-app - :start ((or (:init defaults) identity)) - :stop ((or (:stop defaults) identity))) + :start ((or (:init defaults) identity)) + :stop ((or (:stop defaults) identity))) + +(defroutes oauth-routes + (GET "/oauth/oauth-init" req (oauth-init req)) + (GET "/oauth/oauth-callback" req (oauth-callback req)) + (GET "/logout" req (clear-session! "/"))) + +(defroutes admin-routes + (GET "/admin/enable" req (set-admin! req true)) + (GET "/admin/disable" req (set-admin! req false)) + (POST "/relations" req (create-new-relation! req)) + (POST "/users" req (create-user! req))) (mount/defstate app - :start - (middleware/wrap-base - (routes - (-> #'home-routes - (wrap-routes middleware/wrap-csrf) - (wrap-routes middleware/wrap-formats)) - #'oauth-routes - (-> #'admin-routes - (wrap-routes middleware/wrap-restricted)) - (route/not-found - (:body - (error-page {:status 404 - :title "page not found"})))))) + :start + (middleware/wrap-base + (routes + (-> #'home-routes + (wrap-routes middleware/wrap-csrf) + (wrap-routes middleware/wrap-formats)) + #'oauth-routes + (-> #'admin-routes + (wrap-routes middleware/wrap-restricted)) + (route/not-found + (:body + (error-page {:status 404 + :title "page not found"})))))) diff --git a/src/clj/cat/moauth.clj b/src/clj/cat/moauth.clj index 27655b9..191d020 100644 --- a/src/clj/cat/moauth.clj +++ b/src/clj/cat/moauth.clj @@ -12,19 +12,19 @@ :client-secret (env :oauth-consumer-secret) :authorize-uri (env :authorize-uri) :redirect-uri (str (env :app-host) "/oauth/oauth-callback") - :access-token-uri (env :access-token-uri) - }) + :access-token-uri (env :access-token-uri)}) ; To authorize, redirect the user to the sign in / grant page + (defn- authorize-uri [client-params #_csrf-token] (str - (:authorize-uri client-params) - "?" - (httpclient/generate-query-string {:response_type "code" - :client_id (:client-id client-params) - :redirect_uri (:redirect-uri client-params)}) + (:authorize-uri client-params) + "?" + (httpclient/generate-query-string {:response_type "code" + :client_id (:client-id client-params) + :redirect_uri (:redirect-uri client-params)}) ;"response_type=code" ;"&client_id=" ;(url-encode (:client-id client-params)) @@ -34,7 +34,7 @@ ;(url-encode (:scope client-params)) ;"&state=" ;(url-encode csrf-token) - )) + )) (defn authorize-api-uri "let the user authorize access by redirecting to the signin / grant page @@ -51,20 +51,21 @@ (do (log/debug "Requesting access token with code " code) (let [oauth2-params (oauth2-params) - access-token (httpclient/post (:access-token-uri oauth2-params) + resp (httpclient/post (:access-token-uri oauth2-params) {:form-params {:code code :grant_type "authorization_code" :client_id (:client-id oauth2-params) :client_secret (:client-secret oauth2-params) :redirect_uri (:redirect-uri oauth2-params)} - ;:basic-auth [(:client-id oauth2-params) (:client-secret oauth2-params)] :as :json - :insecure? true - })] - (log/debug "Access token response:" access-token) - (:body access-token))) - (catch Exception e (log/error "Something terrible happened..." e))) - nil)) + :throw-exceptions false + :insecure? true})] + (condp = (:status resp) + 200 (:body resp) + 401 (-> {:status 401 :body "Invalid authentication credentials"}) + {:status 500 :body "Something went pear-shape when trying to authenticate"}))) + ) + (log/info "Invalid csrf token whilst authenticating"))) (defn get-user-info "User info API call" @@ -73,30 +74,31 @@ (-> (httpclient/get url {:oauth-token access-token :as :json :insecure? true}) - :body) - )) + :body))) ; Refresh token when it expires + + (defn- refresh-tokens "Request a new token pair" [refresh-token] (try+ - (let [oauth2-params (oauth2-params) - {{access-token :access_token refresh-token :refresh_token} :body} - (httpclient/post (:access-token-uri oauth2-params) - {:form-params {:grant_type "refresh_token" - :refresh_token refresh-token} - :basic-auth [(:client-id oauth2-params) (:client-secret oauth2-params)] - :as :json - :insecure? true})] - [access-token refresh-token]) - (catch [:status 401] _ nil))) + (let [oauth2-params (oauth2-params) + {{access-token :access_token refresh-token :refresh_token} :body} + (httpclient/post (:access-token-uri oauth2-params) + {:form-params {:grant_type "refresh_token" + :refresh_token refresh-token} + :basic-auth [(:client-id oauth2-params) (:client-secret oauth2-params)] + :as :json + :insecure? true})] + [access-token refresh-token]) + (catch [:status 401] _ nil))) (defn get-fresh-tokens "Returns current token pair if they have not expired, or a refreshed token pair otherwise" [access-token refresh-token] (try+ - (and (get-user-info access-token) - [access-token refresh-token]) - (catch [:status 401] _ (refresh-tokens refresh-token)))) + (and (get-user-info access-token) + [access-token refresh-token]) + (catch [:status 401] _ (refresh-tokens refresh-token)))) diff --git a/src/clj/cat/routes/admin.clj b/src/clj/cat/routes/admin.clj index 862ece4..d60dfd0 100644 --- a/src/clj/cat/routes/admin.clj +++ b/src/clj/cat/routes/admin.clj @@ -1,6 +1,5 @@ (ns cat.routes.admin (:require [cat.db.core :refer [*db*] :as db] - [compojure.core :refer [defroutes GET POST]] [struct.core :as st] [clojure.tools.logging :as log] [ring.util.http-response :as response])) @@ -13,29 +12,28 @@ [[:from_id st/required st/integer-str] [:to_id st/required st/integer-str]]) -(defroutes admin-routes - (GET "/admin/enable" req (-> (response/found "/") - (assoc :session (assoc-in (:session req) [:user :admin :enabled] true)))) - (GET "/admin/disable" req (-> (response/found "/") - (assoc :session (assoc-in (:session req) [:user :admin :enabled] false)))) +(defn set-admin! [req enabled?] + (-> (response/found "/") + (assoc :session (assoc-in (:session req) [:user :admin :enabled] enabled?)))) - (POST "/relations" req - (let [data (:params req) [err result] (st/validate data relation-schema)] - (log/info "Post to " (:uri req)) - (if (nil? err) - (do - (db/create-relation! result) - (response/found "/")) - (do - (response/bad-request "Incorrect input"))))) - (POST "/users" req - (let [data (:params req)] - (log/info "Post to " (:uri req)) - (println data) - (if (st/valid? data user-schema) - (do - (db/create-user! (assoc data :zeusid nil)) - (response/found "/")) - (do - (response/bad-request "Incorrect input"))))) - ) \ No newline at end of file +(defn create-new-relation! [req] + (let [data (:params req) + [err result] (st/validate data relation-schema)] + (if (nil? err) + (do + (log/info "Admin creates relation from " (:from_id data) "to" (:to_id data)) + (db/create-relation! result) + (response/found "/")) + (do + (response/bad-request "Incorrect input"))))) + +(defn create-user! [req] + (let [data (:params req)] + (println data) + (if (st/valid? data user-schema) + (do + (log/info "Admin creates user: " (:name data)) + (db/create-user! (assoc data :zeusid nil)) + (response/found "/")) + (do + (response/bad-request "Incorrect input"))))) diff --git a/src/clj/cat/routes/oauth.clj b/src/clj/cat/routes/oauth.clj index 82b6ea8..b76247b 100644 --- a/src/clj/cat/routes/oauth.clj +++ b/src/clj/cat/routes/oauth.clj @@ -29,7 +29,7 @@ (assoc :session nil))) (defn oauth-init - "Initiates the Twitter OAuth" + "Initiates the OAuth" [request] (let [reee (mo/authorize-api-uri)] (log/debug "authorize uri: " reee) @@ -40,13 +40,14 @@ "Handles the callback from adams with the access_token Fetches the user from the database, creating a new one if not found Sets the user in the session and redirects back to origin \"/\" " - [req_token {:keys [params session]}] + + [{:keys [params session]}] ; oauth request was denied by user (if (:denied params) (-> (found "/") (assoc :flash {:denied true})) ; fetch the request token and do anything else you wanna do if not denied. - (let [{:keys [access_token refresh_token]} (mo/get-authentication-response nil req_token) + (let [{:keys [access_token refresh_token]} (mo/get-authentication-response nil params) fetched-user (mo/get-user-info access_token) local-user (db/get-zeus-user {:zeusid (:id fetched-user)})] (if local-user @@ -78,7 +79,3 @@ ; 401 (println error)))) -(defroutes oauth-routes - (GET "/oauth/oauth-init" req (oauth-init req)) - (GET "/oauth/oauth-callback" [& req_token :as req] (oauth-callback req_token req)) - (GET "/logout" req (clear-session! "/")))