Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions source/oauth/webapp.d
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ class OAuthWebapp
{
version (Have_vibe_d_web) @noRoute:

private OAuthSession loadSessionToContext(scope HTTPServerRequest req, immutable OAuthSettings settings) @safe
in {
assert(settings !is null, "Settings can't be null");
}
body
{
auto session = OAuthSession.load(settings, req.session);
() @trusted {
req.context["oauth.session"] = session;
} ();
return session;
}

/++
Check if a request is from a logged in user

Expand All @@ -49,13 +62,9 @@ class OAuthWebapp
if (!req.session)
return false;

if (auto session =
settings ? OAuthSession.load(settings, req.session) : null)
if (settings !is null)
{
() @trusted {
req.context["oauth.session"] = session;
} ();

loadSessionToContext(req, settings);
return true;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would introduce a serious bug. Should only return true if a session is available.

}

Expand All @@ -82,8 +91,11 @@ class OAuthWebapp
settings = The OAuth settings that apply to this _login attempt
scopes = (Optional) An array of identifiers specifying the scope of
the authorization requested.
Returns: `true` if a OAuth session was obtained and
`false` if no OAuth session is present and a redirect to an
OAuth provider will happen.
+/
void login(
bool login(
scope HTTPServerRequest req,
scope HTTPServerResponse res,
immutable OAuthSettings settings,
Expand All @@ -100,24 +112,27 @@ class OAuthWebapp

auto session = settings.userSession(
req.session, req.query["state"], req.query["code"]);

return true;
}
else
{
if (!req.session)
req.session = res.startSession();

res.redirect(settings.userAuthUri(req.session, extraParams, scopes));
return false;
}
}

/// ditto
void login(
bool login(
scope HTTPServerRequest req,
scope HTTPServerResponse res,
immutable OAuthSettings settings,
in string[] scopes) @safe
{
login(req, res, settings, null, scopes);
return login(req, res, settings, null, scopes);
}

/++
Expand All @@ -131,12 +146,13 @@ class OAuthWebapp

Params:
req = the request to get the relevant session for
settings = The OAuth settings that apply to this _login attempt

Returns: The session associated to req, or `null` if no
session was found.
+/
final
OAuthSession oauthSession(scope HTTPServerRequest req) nothrow @trusted
OAuthSession oauthSession(scope HTTPServerRequest req, immutable OAuthSettings settings = null) nothrow @trusted
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for oauthSession to not take a settings parameter, is that it should be possible for multi-provider apps to get the OAuthSession regardless of the settings used to login.

As mentioned in the documentation comment, oauthSession is just a getter to be used only after checking for session availability and validity.

in
{
try
Expand All @@ -147,15 +163,18 @@ class OAuthWebapp
body
{
try
{
if (auto pCM = "oauth.session" in req.context)
return pCM.get!OAuthSession;
else
return loadSessionToContext(req, settings);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with this, there's still the possibility of oauthSession returning null, i.e. when the user did not login with OAuth. So the need for a null-check will always exist, unless a session is known to be available (e.g. isLoggedIn is known to have returned true for this request).

}
catch (Exception e)
{
import vibe.core.log : logError;
logError("OAuth: Exception occurred while reading request " ~
"context: %s", e.toString());
}

return null;
}
}