Sadly, userinfo endpoints don't need to have sameorigin with the issuer - so we can't protect against forgeries by checking origins

This commit is contained in:
Matthew Scheirer 2018-11-23 13:15:40 -05:00
parent 72b30bd9b0
commit 14680c219f
2 changed files with 2 additions and 12 deletions

View File

@ -238,7 +238,6 @@ impl Display for Expiry {
#[derive(Debug)] #[derive(Debug)]
pub enum Userinfo { pub enum Userinfo {
NoUrl, NoUrl,
MismatchIssuer { expected: String, actual: String },
MismatchSubject { expected: String, actual: String }, MismatchSubject { expected: String, actual: String },
} }
@ -247,7 +246,6 @@ impl ErrorTrait for Userinfo {
use error::Userinfo::*; use error::Userinfo::*;
match *self { match *self {
NoUrl => "No url", NoUrl => "No url",
MismatchIssuer { .. } => "Mismatch issuer",
MismatchSubject { .. } => "Mismatch subject" MismatchSubject { .. } => "Mismatch subject"
} }
} }
@ -262,8 +260,6 @@ impl Display for Userinfo {
use error::Userinfo::*; use error::Userinfo::*;
match *self { match *self {
NoUrl => write!(f, "Config has no userinfo url"), NoUrl => write!(f, "Config has no userinfo url"),
MismatchIssuer { ref expected, ref actual } =>
write!(f, "Token and Userinfo Issuers mismatch: '{}', '{}'", expected, actual),
MismatchSubject { ref expected, ref actual } => MismatchSubject { ref expected, ref actual } =>
write!(f, "Token and Userinfo Subjects mismatch: '{}', '{}'", expected, actual), write!(f, "Token and Userinfo Subjects mismatch: '{}', '{}'", expected, actual),
} }

View File

@ -384,7 +384,6 @@ impl Client {
/// ///
/// - Userinfo::NoUrl if this provider doesn't have a userinfo endpoint /// - Userinfo::NoUrl if this provider doesn't have a userinfo endpoint
/// - Error::Insecure if the userinfo url is not https /// - Error::Insecure if the userinfo url is not https
/// - Userinfo::MismatchIssuer if the userinfo origin does not match the provider's issuer
/// - Error::Jose if the token is not decoded /// - Error::Jose if the token is not decoded
/// - Error::Http if something goes wrong getting the document /// - Error::Http if something goes wrong getting the document
/// - Error::Json if the response is not a valid Userinfo document /// - Error::Json if the response is not a valid Userinfo document
@ -394,11 +393,6 @@ impl Client {
match self.config().userinfo_endpoint { match self.config().userinfo_endpoint {
Some(ref url) => { Some(ref url) => {
discovery::secure(&url)?; discovery::secure(&url)?;
if url.origin() != self.config().issuer.origin() {
let expected = self.config().issuer.as_str().to_string();
let actual = url.as_str().to_string();
return Err(error::Userinfo::MismatchIssuer { expected, actual }.into());
}
let claims = token.id_token.payload()?; let claims = token.id_token.payload()?;
let auth_code = token.access_token().to_string(); let auth_code = token.access_token().to_string();
let mut resp = client.get(url.clone()) let mut resp = client.get(url.clone())
@ -441,7 +435,7 @@ pub struct Options {
/// The userinfo struct contains all possible userinfo fields regardless of scope. [See spec.](https://openid.net/specs/openid-connect-basic-1_0.html#StandardClaims) /// The userinfo struct contains all possible userinfo fields regardless of scope. [See spec.](https://openid.net/specs/openid-connect-basic-1_0.html#StandardClaims)
// TODO is there a way to use claims_supported in config to simplify this struct? // TODO is there a way to use claims_supported in config to simplify this struct?
#[derive(Serialize, Deserialize, Validate)] #[derive(Debug, Serialize, Deserialize, Validate)]
pub struct Userinfo { pub struct Userinfo {
pub sub: String, pub sub: String,
#[serde(default)] pub name: Option<String>, #[serde(default)] pub name: Option<String>,
@ -512,7 +506,7 @@ impl Prompt {
} }
/// Address Claim struct. Can be only formatted, only the rest, or both. /// Address Claim struct. Can be only formatted, only the rest, or both.
#[derive(Serialize, Deserialize)] #[derive(Debug, Serialize, Deserialize)]
pub struct Address { pub struct Address {
#[serde(default)] pub formatted: Option<String>, #[serde(default)] pub formatted: Option<String>,
#[serde(default)] pub street_address: Option<String>, #[serde(default)] pub street_address: Option<String>,