Skip to content

Commit

Permalink
feat: GitHub upstream userinfo custom impl for private E-Mails (#665)
Browse files Browse the repository at this point in the history
* add custom impl for Github private emails

* update Github UI template
  • Loading branch information
sebadob authored Dec 27, 2024
1 parent 9d789c6 commit f1d138f
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@
name: 'Github',
client_id: '',
client_secret: '',
scope: 'read:user',
scope: 'user:email',
root_pem: null,
admin_claim_path: null,
Expand Down
79 changes: 79 additions & 0 deletions src/models/src/entity/auth_provider_cust_impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
use crate::entity::auth_providers::AuthProviderIdClaims;
use rauthy_common::constants::APPLICATION_JSON;
use rauthy_error::{ErrorResponse, ErrorResponseType};
use reqwest::header::{ACCEPT, AUTHORIZATION};
use serde::Deserialize;
use tracing::debug;

#[derive(Debug, Deserialize)]
struct GithubEmailPrivateResponse {
email: String,
primary: bool,
verified: bool,
// The visibility exists as well, but we don't care about it at all - no need to deserialize
// visibility: Option<String>,
}

/// Github is very special and does its own thing, which is super annoying.
/// If a user has no public E-Mail and changed the visibility settings, the
/// user info endpoint will not return any address, even if a valid access token
/// has been provided. In this case, there is another special endpoint only for
/// E-Mail addresses which needs to be used to actually retrieve the address.
/// This means we need a 3rd request to Github.
///
/// Note: The user endpoint is hardcoded because it is very unlikely to ever
/// change in the future. If we would allow this to be customizable, everything
/// would get super messy. If the Github API ever updates, we just need to update
/// the URL here as well.
pub async fn get_update_github_private_email(
client: &reqwest::Client,
access_token: &str,
claims: &mut AuthProviderIdClaims<'_>,
) -> Result<(), ErrorResponse> {
debug!("Trying to get User E-Mail via Github /user/emails endpoint");

let res = client
.get("https://api.github.com/user/emails")
.header(AUTHORIZATION, format!("Bearer {}", access_token))
// .header(ACCEPT, "application/vnd.github+json")
.header(ACCEPT, APPLICATION_JSON)
.header("X-GitHub-Api-Version", "2022-11-28")
.send()
.await?;

let status = res.status().as_u16();
debug!("GET /user/emails status: {}\n{:?}", status, res);

if status < 300 {
let mut emails = res.json::<Vec<GithubEmailPrivateResponse>>().await?;
debug!("GET /user/emails status: {}", status);

if emails.len() == 1 {
let email = emails.swap_remove(0);
claims.email = Some(email.email.into());
claims.email_verified = Some(email.verified);
return Ok(());
}

for email in emails {
if email.primary {
claims.email = Some(email.email.into());
claims.email_verified = Some(email.verified);
return Ok(());
}
}

// TODO is it possible that Github returns multiple E-Mails for a user without
// any of them being flagged as primary?
Err(ErrorResponse::new(
ErrorResponseType::Internal,
"Could not find a the primary user E-Mail in Github response",
))
} else {
let text = res.text().await?;
Err(ErrorResponse::new(
ErrorResponseType::Connection,
format!("Error during User E-Mails lookup: {}", text),
))
}
}
43 changes: 27 additions & 16 deletions src/models/src/entity/auth_providers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::api_cookie::ApiCookie;
use crate::app_state::AppState;
use crate::database::{Cache, DB};
use crate::entity::auth_codes::AuthCode;
use crate::entity::auth_provider_cust_impl;
use crate::entity::clients::Client;
use crate::entity::sessions::Session;
use crate::entity::users::User;
Expand Down Expand Up @@ -52,7 +53,7 @@ use time::OffsetDateTime;
use tracing::{debug, error};
use utoipa::ToSchema;

#[derive(Debug, Clone, Serialize, Deserialize, sqlx::Type)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, sqlx::Type)]
#[sqlx(type_name = "varchar")]
#[sqlx(rename_all = "lowercase")]
#[serde(rename_all = "lowercase")]
Expand Down Expand Up @@ -1000,7 +1001,17 @@ impl AuthProviderCallback {
debug!("GET /userinfo auth provider status: {}", status);

let res_bytes = res.bytes().await?;
let claims = AuthProviderIdClaims::try_from(res_bytes.as_bytes())?;
let mut claims = AuthProviderIdClaims::try_from(res_bytes.as_bytes())?;

if claims.email.is_none() && provider.typ == AuthProviderType::Github {
auth_provider_cust_impl::get_update_github_private_email(
&client,
&access_token,
&mut claims,
)
.await?;
}

claims.validate_update_user(&provider, &link_cookie).await?
} else {
let err = "Neither `access_token` nor `id_token` existed";
Expand Down Expand Up @@ -1183,31 +1194,31 @@ enum ProviderMfaLogin {
}

#[derive(Debug, Deserialize)]
struct AuthProviderIdClaims<'a> {
pub struct AuthProviderIdClaims<'a> {
// pub iss: &'a str,
// json values because some providers provide String, some int
sub: Option<serde_json::Value>,
id: Option<serde_json::Value>,
uid: Option<serde_json::Value>,
pub sub: Option<serde_json::Value>,
pub id: Option<serde_json::Value>,
pub uid: Option<serde_json::Value>,

// aud / azp is not being validated, because it works with OIDC only anyway
// aud: Option<&'a str>,
// azp: Option<&'a str>,
// even though `email` is mandatory for Rauthy, we set it to optional for
// the deserialization to have more control over the error message being returned
email: Option<Cow<'a, str>>,
email_verified: Option<bool>,
pub email: Option<Cow<'a, str>>,
pub email_verified: Option<bool>,

name: Option<Cow<'a, str>>,
given_name: Option<Cow<'a, str>>,
family_name: Option<Cow<'a, str>>,
pub name: Option<Cow<'a, str>>,
pub given_name: Option<Cow<'a, str>>,
pub family_name: Option<Cow<'a, str>>,

address: Option<AuthProviderAddressClaims<'a>>,
birthdate: Option<Cow<'a, str>>,
locale: Option<Cow<'a, str>>,
phone: Option<Cow<'a, str>>,
pub address: Option<AuthProviderAddressClaims<'a>>,
pub birthdate: Option<Cow<'a, str>>,
pub locale: Option<Cow<'a, str>>,
pub phone: Option<Cow<'a, str>>,

json_bytes: Option<&'a [u8]>,
pub json_bytes: Option<&'a [u8]>,
}

impl<'a> TryFrom<&'a [u8]> for AuthProviderIdClaims<'a> {
Expand Down
1 change: 1 addition & 0 deletions src/models/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use sqlx::query;
pub mod api_keys;
pub mod app_version;
pub mod auth_codes;
mod auth_provider_cust_impl;
pub mod auth_providers;
pub mod clients;
pub mod clients_dyn;
Expand Down

0 comments on commit f1d138f

Please sign in to comment.