From 81b249d34196b3645d2e99100bc8fac553dcdcc0 Mon Sep 17 00:00:00 2001
From: Matthieu Bessat <mbess@mbess.net>
Date: Mon, 11 Nov 2024 23:16:50 +0100
Subject: [PATCH] fix: better scope handling

---
 TODO.md                                    | 30 ++++++++++++++++------
 http_integration_tests/authorize.sh        |  2 +-
 http_integration_tests/oauth2c.sh          |  2 +-
 src/controllers/api/oauth2/access_token.rs |  5 ++--
 src/controllers/api/openid/well_known.rs   |  5 ++--
 src/controllers/ui/authorize.rs            | 22 +++++++++++-----
 src/controllers/ui/me.rs                   |  5 ++--
 src/models/authorization.rs                | 12 ++++-----
 src/models/token_claims.rs                 |  6 ++++-
 src/templates/pages/authorize.html         |  2 +-
 10 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/TODO.md b/TODO.md
index e97eac8..1120e53 100644
--- a/TODO.md
+++ b/TODO.md
@@ -2,12 +2,29 @@
 
 - [x] Login form
 - [x] Register form
-- [x] Generate JWT
-- [ ] Redirect to login form if unauthenticated
-- [x] Authorize form
-- [x] Verify authorize
+- [x] Redirect to login form if unauthenticated
 - [x] Upload picture
-- [x] Get access token
+
+- OAuth2
+    - [x] Authorize form
+    - [x] Verify authorize
+    - [x] Get access token
+
+- [x] Support OpenID to use with demo client [oauth2c](https://github.com/cloudentity/oauth2c)
+    - .well-known/openid-configuration
+
+
+- [ ] i18n strings in the http website.
+
+- [ ] App config
+    - Add app logo (URI?)
+
+- [ ] Public endpoint to get user avatar by id
+- [ ] Rework avatar upload to limit size and process the image?
+
+- [ ] Authorize form
+    - Show details about permissions
+    - Show app logo
 
 - [ ] Support error responses by https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1
 
@@ -15,6 +32,3 @@
 - [ ] Add admin panel via API
 - [ ] Add admin CLI
 
-- [ ] Support OpenID to use with demo client [oauth2c](https://github.com/cloudentity/oauth2c)
-
-    - .well-known/openid-configuration
diff --git a/http_integration_tests/authorize.sh b/http_integration_tests/authorize.sh
index da31f3f..615f570 100755
--- a/http_integration_tests/authorize.sh
+++ b/http_integration_tests/authorize.sh
@@ -7,7 +7,7 @@ curl -v http://localhost:8085/authorize \
     -d client_id="a1785786-8be1-443c-9a6f-35feed703609" \
     -d response_type="code" \
     -d redirect_uri="http://localhost:9090/authorize" \
-    -d scope="read_basics" \
+    -d scope="user_read_basic" \
     -d state="qxYAfk4kf6pbZkms78jM"
 
 code="$(cat tmp/headers.txt | grep -i "location" | awk -F ": " '{print $2}' | trurl -f - -g "{query:code}")"
diff --git a/http_integration_tests/oauth2c.sh b/http_integration_tests/oauth2c.sh
index d147067..12069ce 100755
--- a/http_integration_tests/oauth2c.sh
+++ b/http_integration_tests/oauth2c.sh
@@ -7,4 +7,4 @@ oauth2c http://localhost:8085 \
   --response-mode query \
   --grant-type authorization_code \
   --auth-method client_secret_basic \
-  --scopes "read_user_basic"
+  --scopes "user_read_basic"
diff --git a/src/controllers/api/oauth2/access_token.rs b/src/controllers/api/oauth2/access_token.rs
index ab239ba..b0d9c54 100644
--- a/src/controllers/api/oauth2/access_token.rs
+++ b/src/controllers/api/oauth2/access_token.rs
@@ -76,9 +76,10 @@ pub async fn get_access_token(
     // 3. Generate JWT for oauth2 client user session
     let jwt = create_token(
         &app_state.secrets,
-        AppUserTokenClaims::from_client_user_id(
+        AppUserTokenClaims::new(
             &app_client_session.client_id,
-            &authorization.user_id
+            &authorization.user_id,
+            authorization.scopes.to_vec()
         )
     );
     // 4. return JWT
diff --git a/src/controllers/api/openid/well_known.rs b/src/controllers/api/openid/well_known.rs
index 21b9326..64ac5a3 100644
--- a/src/controllers/api/openid/well_known.rs
+++ b/src/controllers/api/openid/well_known.rs
@@ -1,8 +1,9 @@
 use axum::{extract::State, response::IntoResponse, Json};
 use fully_pub::fully_pub;
 use serde::Serialize;
+use strum::IntoEnumIterator;
 
-use crate::server::AppState;
+use crate::{models::authorization::AuthorizationScope, server::AppState};
 
 #[derive(Serialize)]
 #[fully_pub]
@@ -25,7 +26,7 @@ pub async fn get_well_known_openid_configuration(
         authorization_endpoint: format!("{}/authorize", base_url),
         token_endpoint: format!("{}/api/token", base_url),
         userinfo_endpoint: format!("{}/api/user", base_url),
-        scopes_supported: vec!["read_user_basic".into()],
+        scopes_supported: AuthorizationScope::iter().map(|v| v.to_string()).collect(),
         response_types_supported: vec!["code".into()],
         token_endpoint_auth_methods_supported: vec!["client_secret_basic".into()],
     })
diff --git a/src/controllers/ui/authorize.rs b/src/controllers/ui/authorize.rs
index 92cb07e..66e2ee2 100644
--- a/src/controllers/ui/authorize.rs
+++ b/src/controllers/ui/authorize.rs
@@ -87,13 +87,23 @@ pub async fn authorize_form(
             ).into_response();
         }
     };
-
+    // 1.4. Parse and validate scopes
+    let scopes = match parse_scope(&authorization_params.scope) {
+        Ok(v) => v,
+        Err(_err) => {
+            return (
+                StatusCode::BAD_REQUEST,
+                Html("Invalid scopes. Scopes must be space-delimited and snake_case.")
+            ).into_response();
+        }
+    };
     // 2. Check if the app is already authorized
     let authorizations_res = sqlx::query_as::<_, Authorization>(
-        "SELECT * FROM authorizations WHERE user_id = $1 AND client_id = $2"
+        "SELECT * FROM authorizations WHERE user_id = $1 AND client_id = $2 AND scopes = $3"
     )
         .bind(&token_claims.sub)
         .bind(&authorization_params.client_id)
+        .bind(sqlx::types::Json(&scopes))
         .fetch_one(&app_state.db)
         .await;
 
@@ -160,17 +170,17 @@ pub async fn perform_authorize(
             ).into_response();
         }
     };
-    // parse scope again
+    // 1.2. Parse and validate scope to use in DB
     let scopes = match parse_scope(&authorize_form.scope) {
         Ok(v) => v,
-        Err(err) => {
+        Err(_err) => {
             return (
                 StatusCode::BAD_REQUEST,
-                Html(format!("Invalid scope: {}", err))
+                Html("Invalid scopes.")
             ).into_response();
         }
     };
-    // 2. Create an authorizaton code
+    // 2. Create an authorization code
     let authorization_code = get_random_alphanumerical(32);
 
     let authorization = Authorization {
diff --git a/src/controllers/ui/me.rs b/src/controllers/ui/me.rs
index 3b8cb22..3b810a9 100644
--- a/src/controllers/ui/me.rs
+++ b/src/controllers/ui/me.rs
@@ -1,6 +1,7 @@
 use axum::{body::Bytes, extract::State, response::IntoResponse, Extension};
 use axum_typed_multipart::{FieldData, TryFromMultipart, TypedMultipart};
 use fully_pub::fully_pub;
+use log::error;
 use minijinja::context;
 
 use crate::{
@@ -99,11 +100,11 @@ pub async fn me_perform_update_details(
             )
         },
         Err(err) => {
-            dbg!(&err);
+            error!("Cannot update user details. {}", err);
             renderer.render(
                 template_path,
                 context!(
-                    error => Some("Cannot update user details".to_string()),
+                    error => Some("Cannot update user details.".to_string()),
                     user => user_res
                 )
             )
diff --git a/src/models/authorization.rs b/src/models/authorization.rs
index 6762bc2..4713871 100644
--- a/src/models/authorization.rs
+++ b/src/models/authorization.rs
@@ -2,17 +2,17 @@ use fully_pub::fully_pub;
 use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
 use sqlx::types::Json;
-use strum_macros::{Display, EnumString};
+use strum_macros::{Display, EnumIter, EnumString};
 
 #[derive(
     Clone, Debug, Serialize, Deserialize, PartialEq,
     sqlx::Type,
-    Display, EnumString
+    Display, EnumString, EnumIter
 )]
-#[strum(serialize_all = "lowercase")]
-#[fully_pub]
-enum AuthorizationScope {
-    ReadBasics
+#[strum(serialize_all = "snake_case")]
+pub enum AuthorizationScope {
+    UserReadBasic,
+    UserReadRoles
 }
 
 #[derive(sqlx::FromRow, Deserialize, Serialize, Debug)]
diff --git a/src/models/token_claims.rs b/src/models/token_claims.rs
index 1ed624c..3855292 100644
--- a/src/models/token_claims.rs
+++ b/src/models/token_claims.rs
@@ -2,6 +2,8 @@ use fully_pub::fully_pub;
 use jsonwebtoken::get_current_timestamp;
 use serde::{Deserialize, Serialize};
 
+use super::authorization::AuthorizationScope;
+
 #[derive(Debug, Serialize, Deserialize, Clone)]
 #[fully_pub]
 struct UserTokenClaims {
@@ -30,6 +32,7 @@ struct AppUserTokenClaims {
     /// combined subject
     client_id: String,
     user_id: String,
+    scopes: Vec<AuthorizationScope>,
     /// token expiration
     exp: u64,
     /// token issuer
@@ -37,10 +40,11 @@ struct AppUserTokenClaims {
 }
 
 impl AppUserTokenClaims {
-    pub fn from_client_user_id(client_id: &str, user_id: &str) -> Self {
+    pub fn new(client_id: &str, user_id: &str, scopes: Vec<AuthorizationScope>) -> Self {
         AppUserTokenClaims {
             client_id: client_id.into(),
             user_id: user_id.into(),
+            scopes,
             exp: get_current_timestamp() + 86_000,
             iss: "Minauth".into()
         }
diff --git a/src/templates/pages/authorize.html b/src/templates/pages/authorize.html
index a096feb..b4f3566 100644
--- a/src/templates/pages/authorize.html
+++ b/src/templates/pages/authorize.html
@@ -15,7 +15,7 @@
     <ul>
         <li>App name: {{ app.name }}</li>
         <li>App description: <i>{{ app.description }}</i></li>
-        <li>Permisions: {{ authorization_params.scope }}</li>
+        <li>Permissions: {{ authorization_params.scope }}</li>
     </ul>
     <input type="hidden" name="client_id" value="{{ authorization_params.client_id }}" />
     <input type="hidden" name="scope" value="{{ authorization_params.scope }}" />