Skip to content

Commit

Permalink
Merge pull request #1844 from habitat-sh/tracing
Browse files Browse the repository at this point in the history
 allow older user tokens to succesfully authenticate
  • Loading branch information
mwrock authored Sep 13, 2024
2 parents 1d0a75e + 6af03df commit e86ab75
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
10 changes: 9 additions & 1 deletion components/builder-api/src/server/framework/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,12 @@ fn authenticate(token: &str, state: &AppState) -> error::Result<originsrv::Sessi
// Pull the session out of the current token provided so we can validate
// it against the db's tokens
let mut session = AccessToken::validate_access_token(token, &state.config.api.key_path)
.map_err(|_| error::Error::Authorization)?;
.map_err(|e| {
trace!("Unable to validate access token {}, err={:?}", token, e);
error::Error::Authorization
})?;

trace!("Found valid session for {}", token);

if session.get_id() == BUILDER_ACCOUNT_ID {
trace!("Builder token identified");
Expand All @@ -120,6 +125,7 @@ fn authenticate(token: &str, state: &AppState) -> error::Result<originsrv::Sessi

let account = Account::get_by_id(session.get_id() as i64, &conn)
.map_err(error::Error::DieselError)?;
trace!("Found account for token {} in database", token);
session.set_name(account.name);
session.set_email(account.email);

Expand All @@ -128,12 +134,14 @@ fn authenticate(token: &str, state: &AppState) -> error::Result<originsrv::Sessi
}
None => {
// We have no tokens in the database for this user
trace!("Failed to find token {} in database", token);
Err(error::Error::Authorization)
}
}
}
Err(_) => {
// Failed to fetch tokens from the database for this user
trace!("Failed to find tokens for {} in database", token);
Err(error::Error::Authorization)
}
}
Expand Down
28 changes: 24 additions & 4 deletions components/builder-core/src/access_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ impl AccessToken {
/// See the type-level documentation for additional details.
pub fn validate_access_token(token: &str, key_cache: &KeyCache) -> Result<originsrv::Session> {
// Parse the input as an AccessToken.
let token: Self = token.parse()?;
let access_token: Self = token.parse()?;

// Decrypt the contents to get the `originsrv::AccessToken`
// protobuf inside.
let payload = token.decrypt(key_cache)?;
let payload = access_token.decrypt(key_cache)?;

// Ensure that the token has not expired yet.
//
Expand All @@ -112,7 +112,24 @@ impl AccessToken {
return Err(Error::TokenExpired);
}
}
_ => return Err(Error::TokenInvalid),
_ => {
// mwrock - 2024-09-13
// Prior to the chronos update 4 months ago this is the max timestamp used for user
// tokens We have succesfully parsed this value via the call to
// Utc.timestamp_opt above for years Yesterday (2024-09-12), this
// value became out of bounds and yields a 401 trying to auth
// Right now we have no idea why and some day perhaps we will all laugh around the
// fire as we remember this bug and how trivial it truly was. Today
// no one is laughing. I hope there will be a better fix than this
// in the near future but this will allow the many keys currently
// out in the wild with this value to authenticate.
if payload.get_expires() != 8_210_298_326_400 {
trace!("unable to parse timestamp from expires {} for token {}",
payload.get_expires(),
token);
return Err(Error::TokenInvalid);
}
}
}

// If all is OK, finally convert into an `originsrv::Session`.
Expand Down Expand Up @@ -219,12 +236,15 @@ impl FromStr for AccessToken {
//
// See documentation comments on builder_core::crypto::encrypt as
// well.
if encrypted.parse::<SignedBox>().is_err() {
if let Err(err) = encrypted.parse::<SignedBox>() {
trace!("unable to parse SignedBox from token {}, err: {}", s, err);
Err(Error::TokenInvalid)
} else {
Ok(Self(encrypted))
}
} else {
trace!("token {} is not prefixed with an underscore and is invalid",
s);
Err(Error::TokenInvalid)
}
}
Expand Down

0 comments on commit e86ab75

Please sign in to comment.