Skip to content

Commit

Permalink
Refactor token handling and improve error checks
Browse files Browse the repository at this point in the history
Revised the token management logic to address inconsistencies in token revocation and validation. Updated tests to better reflect actual use cases and ensure robustness. Specifically, enhanced JWT token error handling and aligned the token structure with expected authentication flows. These changes improve reliability and maintainability of the authentication module.
  • Loading branch information
eliasjpr committed Oct 11, 2024
1 parent f75129a commit 617e8b6
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 44 deletions.
24 changes: 13 additions & 11 deletions spec/authly_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe Authly do
end

it "returns access_token for AuthorizationCode grant" do
code = Authly.code("code", client_id, redirect_uri, scope, state, code_challenge, code_challenge_method).to_s
code = Authly.code("code", client_id, redirect_uri, "read", state, code_challenge, code_challenge_method).to_s
token = Authly.access_token(
grant_type: "authorization_code",
client_id: client_id,
Expand Down Expand Up @@ -80,18 +80,20 @@ describe Authly do

describe ".revoke" do
it "revokes token" do
a_token = Authly::AccessToken.new(client_id, scope)
Authly.revoke(a_token.jti)
Authly.revoked?(a_token.jti).should eq true
a_token = Authly::AccessToken.new(client_id, scope).access_token
Authly.revoke(a_token)
Authly.revoked?(a_token).should eq true
end

it "does not revoke token" do
a_token = Authly::AccessToken.new(client_id, scope)
Authly.revoked?(a_token.jti).should eq false
a_token = Authly::AccessToken.new(client_id, scope).access_token
Authly.revoked?(a_token).should eq false
end

it "does not revoke token" do
Authly.revoked?("invalid_jti").should eq false
it "raises error for invalid jwt token" do
expect_raises JWT::DecodeError do
Authly.revoked?("invalid_jti")
end
end
end

Expand All @@ -106,16 +108,16 @@ describe Authly do
a_token = Authly::AccessToken.new(client_id, scope)
token = a_token.access_token

Authly.revoke(a_token.jti)
Authly.revoke(token)

Authly.valid?(token).should eq false
end

it "returns false" do
a_token = Authly::AccessToken.new(client_id, scope)
token = a_token.access_token
Authly.revoke(a_token.jti)
Authly.valid?(token + "invalid").should eq false
Authly.revoke(token)
Authly.valid?(token).should eq false
end
end

Expand Down
16 changes: 10 additions & 6 deletions spec/grant_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@ module Authly
client_secret = "secret"
username = "username"
password = "password"
redirect_uri = "hhttps://www.example.com/callback"
refresh_token = "test_refresh_token"
redirect_uri = "https://www.example.com/callback"
refresh_token = Authly.jwt_encode({
"jti" => Random::Secure.hex(32),
"sub" => client_id,
"name" => "refresh token",
"iat" => Time.utc.to_unix,
"iss" => Authly.config.issuer,
"exp" => Authly.config.refresh_ttl.from_now.to_unix,
})
authorization_code = Authly::Code.new(client_id, "read", redirect_uri, "", "", username).to_s

it "creates an access token with valid client credentials grant" do
Expand Down Expand Up @@ -46,8 +53,6 @@ module Authly
end

it "creates an access token with valid refresh token grant" do
authorization_code = Authly::Code.new(client_id, "read", redirect_uri, "", "", username).to_s

grant = Grant.new("refresh_token",
client_id: client_id,
client_secret: client_secret,
Expand All @@ -57,7 +62,7 @@ module Authly
token = grant.token

token.client_id.should eq client_id
token.scope.should eq ""
token.scope.should eq "read"
end

it "raises error for unsupported grant type" do
Expand All @@ -71,7 +76,6 @@ module Authly
redirect_uri: redirect_uri,
code: authorization_code)

grant.code = Authly.jwt_encode({"scope" => "read"})
token = grant.token

token.scope.should eq "read"
Expand Down
3 changes: 2 additions & 1 deletion spec/support/handlers_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ module Authly

describe "RevokeHandler" do
it "returns success message for valid token revocation" do
response = HTTP::Client.post("#{BASE_URI}/revoke", form: {"token" => "valid_token"})
access_token = Authly::AccessToken.new( "1", "read").access_token
response = HTTP::Client.post("#{BASE_URI}/revoke", form: {"token" => access_token})
response.status_code.should eq 200
response.body.should eq "Token revoked successfully"
end
Expand Down
1 change: 1 addition & 0 deletions src/authly/access_token.cr
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ module Authly

def refresh_token
Authly.jwt_encode({
"jti" => Random::Secure.hex(32),
"sub" => @client_id,
"name" => "refresh token",
"iat" => Time.utc.to_unix,
Expand Down
1 change: 1 addition & 0 deletions src/authly/code.cr
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module Authly

def jwt
Authly.jwt_encode({
"jti" => Random::Secure.hex(32),
"code" => code,
"challenge" => challenge,
"method" => method,
Expand Down
7 changes: 5 additions & 2 deletions src/authly/grant.cr
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ module Authly

private def generate_id_token
if scope.includes? "openid"
Authly.jwt_encode Authly.owners.id_token auth_code["user_id"].as_s
payload = Authly.owners.id_token(auth_code["user_id"].as_s)
payload["iss"] = Authly.config.issuer
Authly.jwt_encode(payload)
end
end

Expand All @@ -87,14 +89,15 @@ module Authly
end

private def validate_scope!
return if scope.empty?
unless Authly.clients.allowed_scopes?(@client_id, scope)
raise Error.invalid_scope
end
end

private def revoke_old_refresh_token(token : String)
if @grant_strategy.is_a?(RefreshToken)
@token_manager.revoke(@refresh_token)
@token_manager.revoke(@refresh_token)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/authly/handler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Authly
AuthorizationHandler.handle(context)
when "/oauth/token"
return call_next(context) unless context.request.method == "POST"
if context.request.params["grant_type"] == "refresh_token"
if context.request.form_params["grant_type"] == "refresh_token"
RefreshTokenHandler.handle(context)
else
AccessTokenHandler.handle(context)
Expand Down
2 changes: 0 additions & 2 deletions src/authly/handlers/authorization_handler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ module Authly
return
end

puts "State: #{STATE_STORE.valid?(state)} - #{state}"

# Generate authorization code after user consent
authorization_code = Authly.code(
response_type,
Expand Down
29 changes: 13 additions & 16 deletions src/authly/handlers/refresh_token_handler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ module Authly
class RefreshTokenHandler
def self.handle(context : HTTP::Server::Context)
# Extract parameters from the request
client_id = context.request.params["client_id"]?
client_secret = context.request.params["client_secret"]?
refresh_token = context.request.params["refresh_token"]?
client_id = context.request.form_params["client_id"]?
client_secret = context.request.form_params["client_secret"]?
refresh_token = context.request.form_params["refresh_token"]?

# Ensure all required parameters are present
unless client_id && client_secret && refresh_token
Expand All @@ -13,19 +13,16 @@ module Authly
end

# Instantiate the RefreshToken service
refresh_token_service = Authly::RefreshToken.new(client_id, client_secret, refresh_token)

# Authorize and refresh the token
if refresh_token_service.authorized?
new_refresh_token = refresh_token_service.refresh_access_token
new_access_token = Authly.access_token("refresh_token", client_id: client_id, client_secret: client_secret)

# Respond with new tokens
ResponseHelper.write(context, 200, "application/json", {"refresh_token" => new_refresh_token, "access_token" => new_access_token}.to_json)
else
# Respond with unauthorized error
ResponseHelper.write(context, 401, "application/json", {"error" => "unauthorized_client"}.to_json)
end
token = Authly.access_token("refresh_token", client_id: client_id, client_secret: client_secret)
ResponseHelper.write(
context,
200,
"application/json",
{
refresh_token: token.refresh_token,
access_token: token.access_token,
}.to_json
)
rescue e : Error
ResponseHelper.write(context, e.code, "application/json", {"error" => e.message}.to_json)
end
Expand Down
9 changes: 4 additions & 5 deletions src/authly/token_manager.cr
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,17 @@ module Authly
@config.token_store.revoke(jti)
end

def revoked?(token) : Bool
def revoked?(token : String) : Bool
decoded_token, _header = Authly.jwt_decode(token)
jti = decoded_token["jti"].to_s
@config.token_store.revoked?(jti)
end

def valid?(token) : Bool
decoded_token, _header = Authly.jwt_decode(token)
jti = decoded_token["jti"].to_s
return false if revoked?(jti)
return false if revoked?(token)

exp = decoded_token["exp"].to_s.to_i
exp = decoded_token["exp"].to_s.to_i64
Time.utc.to_unix < exp
rescue e : JWT::DecodeError
Log.error { "Invalid token - #{e.message}" }
Expand All @@ -55,7 +54,7 @@ module Authly
payload, _header = Authly.jwt_decode(token)

# Check if the token is expired (exp claim is typically in seconds since epoch)
if Time.local.to_unix > payload["exp"].to_s.to_i
if Time.local.to_unix > payload["exp"].to_s.to_i64
return {"active" => false, "exp" => payload["exp"].as_i64}
end

Expand Down

0 comments on commit 617e8b6

Please sign in to comment.