Skip to content

Commit

Permalink
Fix never expiring access tokens and codes
Browse files Browse the repository at this point in the history 
This number was being encoded as a binary, and later compared to an
integer.  Erlang term comparison rules mean that the time of
expiration was always greater than the current time.

The 'expiry_time' parameter in a grant context is now an integer
rather than a binary.  This may be a breaking change for anyone
relying on this value.

A side issue here was that the mocks in the TTL tests were returning
bad results that would trigger the expected results.
  • Loading branch information
@danielwhite
danielwhite committed Jan 17, 2014
1 parent 7e644f8 commit 2b69523
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 101 deletions.
2 changes: 1 addition & 1 deletion src/oauth2.erl
View file
Expand Up @@ -322,7 +322,7 @@ verify_access_token(AccessToken, AppCtx1) ->
build_context(Client, ExpiryTime, ResOwner, Scope) ->
[ {<<"client">>, Client}
, {<<"resource_owner">>, ResOwner}
, {<<"expiry_time">>, list_to_binary(integer_to_list(ExpiryTime))}
, {<<"expiry_time">>, ExpiryTime}
, {<<"scope">>, Scope} ].

-spec seconds_since_epoch(integer()) -> non_neg_integer().
Expand Down
193 changes: 93 additions & 100 deletions test/oauth2_tests.erl
View file
Expand Up @@ -111,64 +111,48 @@ bad_authorize_client_credentials_test_() ->

bad_ttl_test_() ->
{setup,
fun () ->
meck:new(oauth2_mock_backend),
meck:expect(oauth2_mock_backend,
authenticate_client,
fun(_, _, _) -> {ok, {client, 4711}}
end),
meck:expect(oauth2_mock_backend,
resolve_access_code,
fun(_, _) -> {ok, [{<<"identity">>, <<"123">>},
{<<"resource_owner">>, <<>>},
{<<"expiry_time">>, 123},
{<<"scope">>, <<>>}]}
end),
meck:expect(oauth2_mock_backend,
revoke_access_code,
fun(_, _) -> ok end),
meck:expect(oauth2_mock_backend,
resolve_access_token,
fun(_, _) -> {ok, [{<<"identity">>, <<"123">>},
{<<"resource_owner">>, <<>>},
{<<"expiry_time">>, 123},
{<<"scope">>, <<>>}]}
end),
meck:expect(oauth2_mock_backend,
revoke_access_token,
fun(_, _) -> ok end),
meck:expect(oauth2_mock_backend,
resolve_refresh_token,
fun(_, _) -> {ok, [{<<"identity">>, <<"123">>},
{<<"resource_owner">>, <<>>},
{<<"expiry_time">>, 123},
{<<"scope">>, <<>>}]}
end),
meck:expect(oauth2_mock_backend, revoke_refresh_token, fun(_, _) -> ok end),
ok
end,
fun (_) ->
meck:unload(oauth2_mock_backend)
end,
fun(_) ->
[
?_assertMatch({error, invalid_grant},
oauth2:verify_access_code(
<<"XoaUdYODRCMyLkdaKkqlmhsl9QQJ4b">>,
foo_context)),
?_assertMatch({error, access_denied},
oauth2:verify_access_token(
<<"TiaUdYODLOMyLkdaKkqlmdhsl9QJ94a">>,
foo_context)),
?_assertMatch({error, invalid_grant},
oauth2:refresh_access_token(
?CLIENT_ID,
?CLIENT_SECRET,
<<"TiaUdYODLOMyLkdaKkqlmdhsl9QJ94a">>,
?CLIENT_SCOPE,
foo_context))
]
end}.
fun start/0,
fun stop/1,
fun(_) ->
[
fun() ->
application:set_env(oauth2, expiry_time, 0),

{ok, Response} = issue_access_token(foo_context),
{ok, Token} = oauth2_response:access_token(Response),

?assertEqual({error, access_denied},
oauth2:verify_access_token(Token, foo_context))
end,
fun() ->
application:set_env(oauth2, expiry_time, 0),

{ok, Response} = issue_access_code(foo_context),
{ok, Code} = oauth2_response:access_code(Response),

?assertEqual({error, invalid_grant},
oauth2:verify_access_code(Code, foo_context))
end,
fun() ->
application:set_env(oauth2, expiry_time, 3600),

{ok, Res1} = issue_access_code(foo_context),

application:set_env(oauth2, expiry_time, 0),

{ok, Res2} = issue_token_and_refresh(Res1, foo_context),

{ok, RefreshToken} = oauth2_response:refresh_token(Res2),
?assertEqual({error, invalid_grant},
oauth2:refresh_access_token(
?CLIENT_ID,
?CLIENT_SECRET,
RefreshToken,
?USER_SCOPE,
foo_context))
end
]
end}.

verify_access_token_test_() ->
{setup,
Expand All @@ -177,18 +161,11 @@ verify_access_token_test_() ->
fun(_) ->
[
fun() ->
{ok, {foo_context, Authorization}} =
oauth2:authorize_client_credentials(
?CLIENT_ID,
?CLIENT_SECRET,
?CLIENT_SCOPE,
foo_context),
{ok, {foo_context, Response}} =
oauth2:issue_token(Authorization, foo_context),
{ok, Response} = issue_access_token(foo_context),
{ok, Token} = oauth2_response:access_token(Response),
?assertMatch( {ok, {foo_context, _}}
, oauth2:verify_access_token( Token
, foo_context ))

?assertMatch({ok, {foo_context, _}},
oauth2:verify_access_token(Token, foo_context))
end,
?_assertMatch({error, access_denied},
oauth2:verify_access_token(<<"nonexistent_token">>,
Expand Down Expand Up @@ -241,7 +218,7 @@ bad_access_code_test_() ->
?CLIENT_SCOPE,
foo_context),
?_assertMatch({error, invalid_grant},
oauth2:verify_access_code(<<"nonexistent_token">>))
oauth2:verify_access_code(<<"nonexistent_token">>, foo_context))
end
]
end}.
Expand All @@ -253,16 +230,7 @@ verify_access_code_test_() ->
fun(_) ->
[
fun() ->
{ok, {foo_context, Auth}} =
oauth2:authorize_code_request(
?CLIENT_ID,
?CLIENT_URI,
?USER_NAME,
?USER_PASSWORD,
?USER_SCOPE,
foo_context),
{ok, {foo_context, Response}} =
oauth2:issue_code(Auth, foo_context),
{ok, Response} = issue_access_code(foo_context),
{ok, Code} = oauth2_response:access_code(Response),
?assertMatch({ok, {user, 31337}},
oauth2_response:resource_owner(Response)),
Expand Down Expand Up @@ -352,26 +320,8 @@ verify_refresh_token_test_() ->
fun(_) ->
[
fun() ->
{ok, {foo_context, Auth}} =
oauth2:authorize_code_request(
?CLIENT_ID,
?CLIENT_URI,
?USER_NAME,
?USER_PASSWORD,
?USER_SCOPE,
foo_context),
{ok, {foo_context, Response}} =
oauth2:issue_code(Auth, foo_context),
{ok, Code} = oauth2_response:access_code(Response),
{ok, {foo_context, Auth2}} =
oauth2:authorize_code_grant(
?CLIENT_ID,
?CLIENT_SECRET,
Code,
?CLIENT_URI,
foo_context),
{ok, {foo_context, Res2}} =
oauth2:issue_token_and_refresh(Auth2, foo_context),
{ok, Res1} = issue_access_code(foo_context),
{ok, Res2} = issue_token_and_refresh(Res1, foo_context),
{ok, RefreshToken} = oauth2_response:refresh_token(Res2),
{ok, _} = oauth2:refresh_access_token(
?CLIENT_ID,
Expand Down Expand Up @@ -400,3 +350,46 @@ start() ->
stop(_State) ->
oauth2_mock_backend:stop(),
ok.


%%%===================================================================
%%% Helpers
%%%===================================================================

issue_access_token(Context) ->
{ok, {Context, Authorization}} =
oauth2:authorize_client_credentials(
?CLIENT_ID,
?CLIENT_SECRET,
?CLIENT_SCOPE,
Context),
{ok, {Context, Response}} =
oauth2:issue_token(Authorization, Context),
{ok, Response}.


issue_access_code(Context) ->
{ok, {Context, Auth}} =
oauth2:authorize_code_request(
?CLIENT_ID,
?CLIENT_URI,
?USER_NAME,
?USER_PASSWORD,
?USER_SCOPE,
Context),
{ok, {Context, Response}} =
oauth2:issue_code(Auth, Context),
{ok, Response}.

issue_token_and_refresh(Response, Context) ->
{ok, Code} = oauth2_response:access_code(Response),
{ok, {Context, Auth2}} =
oauth2:authorize_code_grant(
?CLIENT_ID,
?CLIENT_SECRET,
Code,
?CLIENT_URI,
Context),
{ok, {Context, Res2}} =
oauth2:issue_token_and_refresh(Auth2, Context),
{ok, Res2}.

0 comments on commit 2b69523

Please sign in to comment.