Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Implement Shopify provider
  • Loading branch information
ChrisInSeattle committed Dec 19, 2017
commit c195be8947951543e5832f50e91a335fd799b81d
26 changes: 18 additions & 8 deletions samples/Mvc.Client/Controllers/AuthenticationController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,31 @@ public async Task<IActionResult> SignIn([FromForm] string provider)
// Instruct the middleware corresponding to the requested external identity
// provider to redirect the user agent to its own authorization endpoint.
// Note: the authenticationScheme parameter must match the value configured in Startup.cs
if (provider == ShopifyAuthenticationDefaults.AuthenticationScheme)
if (provider != ShopifyAuthenticationDefaults.AuthenticationScheme)
{
var authProps = new AuthenticationProperties
return Challenge(new AuthenticationProperties
{
RedirectUri = "/",
Items = { [ShopifyAuthenticationDefaults.ShopNameAuthenticationProperty] = "the-cat-ball-test" }
};

return Challenge(authProps, provider);
}, provider);
}

return Challenge(new AuthenticationProperties

// Shopify OAuth differs from most (all?) others in that you need to know the host name of the
// shop in order to construct the authorization endpoint. This can be aquired either from the
// user directly, or provided by the shopify app store during app install/activation.
var authProps = new ShopifyAuthenticationProperties("the-cat-ball-test") // Put your shop name here.
{
RedirectUri = "/",
}, provider);

// Override OAuthOptions.Scope. Must be fully formatted.
//Scope = "read_customers,read_orders"

// Set to true for a per-user, online-only, token. The retured token has an expiration date
// and should not be persisted. An offline token is requested by default.
//RequestPerUserToken = true
};

return Challenge(authProps, provider);
}

[HttpGet("~/signout"), HttpPost("~/signout")]
Expand Down
3 changes: 3 additions & 0 deletions samples/Mvc.Client/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* for more information concerning the license and the contributors participating to this project.
*/

using System.Collections.Generic;
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -49,6 +50,8 @@ public void ConfigureServices(IServiceCollection services)
options.ApiKey = "275f6ff4f18729971335761c6a025bfe";
options.ApiSecretKey = "35b8276265bfbda78fe8af7a89805792";

options.Scope.Add("read_customers");
options.Scope.Add("read_orders");
});

services.AddMvc();
Expand Down
47 changes: 45 additions & 2 deletions src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationDefaults.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@

namespace AspNet.Security.OAuth.Shopify
{
/// <summary>
/// Default values used by the Shopify authentication middleware.
/// </summary>
public static class ShopifyAuthenticationDefaults
{
/// <summary>
Expand Down Expand Up @@ -44,11 +47,51 @@ public static class ShopifyAuthenticationDefaults
/// <summary>
/// Default value for <see cref="OAuthOptions.UserInformationEndpoint"/>.
/// </summary>
public const string UserInformationEndpoint = "https://{0}.myshopify.com/admin";
public const string UserInformationEndpoint = "https://{0}.myshopify.com/admin/shop";


Copy link
Member

Choose a reason for hiding this comment

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

Nit: unnecessary new line.

/// <summary>
///
/// Name of dictionary entry in <see cref="AuthenticationProperties.Items"/> that contains
/// the name of the shop.
/// </summary>
public const string ShopNameAuthenticationProperty = "ShopName";

/// <summary>
/// Set this authentication property to override the scope set in <see cref="OAuthOptions.Scope"/>. Note - if this
/// override is used, it must be fully formatted.
/// </summary>
public const string ShopScopeAuthenticationProperty = "Scope";

/// <summary>
/// Additional grant options. The only acceptable value is "per-user"
/// </summary>
public const string GrantOptionsAuthenticationProperty = "GrantOptions";

/// <summary>
/// Per user is the only acceptable grant option at this time.
Copy link
Member

Choose a reason for hiding this comment

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

Is this still true?

/// </summary>
public const string PerUserAuthenticationPropertyValue = "per-user";


Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

/// <summary>
/// The claim type which contains the permission scope returned by Shopify during authorization.
/// This may not be the same scope requested, so apps should verify they have the scope they need.
/// </summary>
public const string ShopifyScopeClaimType = "urn:shopify:scope";

/// <summary>
/// The plan name that this shop is using.
/// </summary>
public const string ShopifyPlanNameClaimType = "urn:shopify:plan_name";

/// <summary>
/// Claim type indicating whether or not this shop if eligable to make payments.
Copy link
Member

Choose a reason for hiding this comment

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

Typo: eligible

/// </summary>
public const string ShopifyEligableForPaymentsClaimType = "urn:shopify:eligible_for_payments";
Copy link
Member

Choose a reason for hiding this comment

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

Typo: ShopifyEligibleForPaymentsClaimType


/// <summary>
/// The timezone that that the shop is using.
/// </summary>
public const string ShopifyTimezoneClaimType = "urn:shopify:timezone";
}
}
115 changes: 91 additions & 24 deletions src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
* for more information concerning the license and the contributors participating to this project.
*/

using System;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Security.Claims;
using System.Text.Encodings.Web;
Expand All @@ -18,12 +18,16 @@
using Microsoft.AspNetCore.WebUtilities;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

namespace AspNet.Security.OAuth.Shopify
{
/// <inheritdoc />
Copy link
Member

Choose a reason for hiding this comment

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

A more specific comment is better as it's for Shopify, not any-old-OAuth.

[UsedImplicitly]
public class ShopifyAuthenticationHandler : OAuthHandler<ShopifyAuthenticationOptions>
{
/// <inheritdoc />
public ShopifyAuthenticationHandler(
[NotNull] IOptionsMonitor<ShopifyAuthenticationOptions> options,
[NotNull] ILoggerFactory logger,
Expand All @@ -33,12 +37,13 @@ public ShopifyAuthenticationHandler(
{
}

/// <inheritdoc />
protected override async Task<AuthenticationTicket> CreateTicketAsync(
[NotNull] ClaimsIdentity identity,
[NotNull] AuthenticationProperties properties,
[NotNull] OAuthTokenResponse tokens)
{
var uri = string.Format(ShopifyAuthenticationDefaults.UserInformationEndpoint + "/shop",
var uri = string.Format(ShopifyAuthenticationDefaults.UserInformationEndpoint,
properties.Items[ShopifyAuthenticationDefaults.ShopNameAuthenticationProperty]);

var request = new HttpRequestMessage(HttpMethod.Get, uri);
Expand All @@ -59,24 +64,51 @@ protected override async Task<AuthenticationTicket> CreateTicketAsync(

var payload = JObject.Parse(await response.Content.ReadAsStringAsync());

#if DEBUG
// ReSharper disable once UnusedVariable
var jsonStr = payload.ToString(Formatting.Indented);
Copy link
Member

Choose a reason for hiding this comment

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

Remove this please.

#endif

// In Shopify, the customer can modify the scope given to the app. Apps should verify
// that the customer is allowing the required scope.
var actualScope = tokens.Response["scope"].ToString();
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to access scope in a safer way (e.g. if it's not there or is null)?

identity.AddClaim(new Claim("urn:shopify:scope", actualScope));
var isPersistent = true;

// If the request was for a "per-user" (i.e. no offline access)
if (tokens.Response.TryGetValue("expires_in", out var val))
Copy link
Member

Choose a reason for hiding this comment

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

Better local name than val please.

{
isPersistent = false;

var expires = DateTimeOffset.Now.AddSeconds(Convert.ToInt32(val.ToString()));
Copy link
Member

Choose a reason for hiding this comment

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

Use UtcNow.

Copy link
Member

Choose a reason for hiding this comment

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

Use CultureInfo.InvariantCulture for the calls to ToString() (I can't tell what the type is because you've used var so that might not be valid) and Convert.ToInt32() (int.TryParse() might be better candidate).

identity.AddClaim(new Claim(ClaimTypes.Expiration, expires.ToString("O"), "dateTime"));

actualScope = tokens.Response["associated_user_scope"].ToString();
Copy link
Member

Choose a reason for hiding this comment

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

What is the type of these items being ToString()'d?


var userData = tokens.Response["associated_user"].ToString();
identity.AddClaim(new Claim(ClaimTypes.UserData, userData, "json"));
}

identity.AddClaim(new Claim(ClaimTypes.IsPersistent, isPersistent.ToString(), "boolean"));
identity.AddClaim(new Claim(ShopifyAuthenticationDefaults.ShopifyScopeClaimType, actualScope));

var principal = new ClaimsPrincipal(identity);
var context = new OAuthCreatingTicketContext(principal, properties, Context, Scheme, Options, Backchannel, tokens, payload);

var context = new OAuthCreatingTicketContext(principal, properties, Context, Scheme, Options, Backchannel, tokens, payload);
context.RunClaimActions(payload);
Copy link
Member

Choose a reason for hiding this comment

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

This can just be context.RunClaimActions().


await Options.Events.CreatingTicket(context);
var ticket = new AuthenticationTicket(context.Principal, context.Properties, Scheme.Name);

return ticket;
}

/// <inheritdoc />
protected override string FormatScope()
{
return Options.ShopifyScope;
return string.Join(",", Options.Scope);
}

/// <inheritdoc />
protected override string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri)
{
if (!properties.Items.ContainsKey(ShopifyAuthenticationDefaults.ShopNameAuthenticationProperty))
Expand All @@ -91,32 +123,68 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties,
var shopName = properties.Items[ShopifyAuthenticationDefaults.ShopNameAuthenticationProperty];
var uri = string.Format(Options.AuthorizationEndpoint, shopName);

// Get the permission scope, which can either be set in options or overridden in AuthenticationProperties.
var scope = properties.Items.ContainsKey(ShopifyAuthenticationDefaults.ShopScopeAuthenticationProperty) ?
Copy link
Member

Choose a reason for hiding this comment

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

TryGetValue() would be more efficient than using ContainsKey() + [].

properties.Items[ShopifyAuthenticationDefaults.ShopScopeAuthenticationProperty] :
FormatScope();

var url = QueryHelpers.AddQueryString(uri, new Dictionary<string, string>
{
["client_id"] = Options.ApiKey ?? Options.ClientId,
["scope"] = FormatScope(),
["client_id"] = Options.ClientId,
["scope"] = scope,
["redirect_uri"] = redirectUri,
["state"] = Options.StateDataFormat.Protect(properties),
["state"] = Options.StateDataFormat.Protect(properties)
});

// If we're requesting a per-user, online only, token, add the grant_options query param.
if (properties.Items.ContainsKey(ShopifyAuthenticationDefaults.GrantOptionsAuthenticationProperty))
Copy link
Member

Choose a reason for hiding this comment

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

TryGetValue() would be more efficient than using ContainsKey() + [].

{
var grantOptions = properties.Items[ShopifyAuthenticationDefaults.GrantOptionsAuthenticationProperty];
if (grantOptions != null &&
grantOptions.Equals(ShopifyAuthenticationDefaults.PerUserAuthenticationPropertyValue))
Copy link
Member

Choose a reason for hiding this comment

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

Assuming this is a string, use string.Equals and explicitly state whether it is case sensitive or not using StringComparisonOptions.Ordinal..., then the null check also becomes redundant.

{
url = QueryHelpers.AddQueryString(url, "grant_options[]",
ShopifyAuthenticationDefaults.PerUserAuthenticationPropertyValue);
}
}

return url;
}


/// <inheritdoc />
protected override async Task<OAuthTokenResponse> ExchangeCodeAsync(
[NotNull] string code,
[NotNull] string redirectUri)
{
var shop = Context.Request.Query["shop"];
var hmac = Context.Request.Query["hmac"];
var state = Context.Request.Query["state"];

var shopDns = shop.ToString().Split('.').First();
var z = Options.StateDataFormat.Unprotect(state);
if (!z.Items[ShopifyAuthenticationDefaults.ShopNameAuthenticationProperty]
.Equals(shopDns, StringComparison.InvariantCultureIgnoreCase))
string shopDns;

try
{
var shop = Context.Request.Query["shop"];
var state = Context.Request.Query["state"];

// Shop name must end with myshopify.com
if (!shop.ToString().EndsWith(".myshopify.com"))
Copy link
Member

Choose a reason for hiding this comment

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

Specify whether this is case sensitive or not explicitly via StringComparisonOptions.Ordinal....

{
throw new Exception("Unexpected query string.");
}

// Strip out the "myshopify.com" suffix
shopDns = shop.ToString().Split('.').First();
Copy link
Member

Choose a reason for hiding this comment

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

This is the second time shop.ToString() is called - you can cache it into a local variable.

Copy link
Member

Choose a reason for hiding this comment

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

[0] rather than .First()?


// Verify that the shop name encoded in "state" matches the shop name we used to
// request the token. This probably isn't necessary, but it's an easy extra verification.
var z = Options.StateDataFormat.Unprotect(state);
Copy link
Member

Choose a reason for hiding this comment

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

Better name than z please.

if (!z.Items[ShopifyAuthenticationDefaults.ShopNameAuthenticationProperty]
Copy link
Member

Choose a reason for hiding this comment

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

Use the static string.Equals() method as it is null safe.

.Equals(shopDns, StringComparison.InvariantCultureIgnoreCase))
{
throw new Exception("Unexpected query string");
Copy link
Member

Choose a reason for hiding this comment

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

Is there a better exception type you can use?

Copy link
Member

Choose a reason for hiding this comment

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

Tell the integrating developer why it's "unexpected"? Maybe explicitly state that the shop is mismatched?

}
}
catch (Exception e)
{

Logger.LogError("An error occurred while exchanging tokens: " + e.Message);
return OAuthTokenResponse.Failed(e);
}

var uri = string.Format(Options.TokenEndpoint, shopDns);
Copy link
Member

Choose a reason for hiding this comment

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

CultureInfo.InvariantCulture?

Expand All @@ -125,9 +193,9 @@ protected override async Task<OAuthTokenResponse> ExchangeCodeAsync(

request.Content = new FormUrlEncodedContent(new Dictionary<string, string>
{
["client_id"] = Options.ApiKey ?? Options.ClientId,
["client_secret"] = Options.ApiSecretKey ?? Options.ClientSecret,
["code"] = code,
["client_id"] = Options.ClientId,
["client_secret"] = Options.ClientSecret,
["code"] = code
});

var response = await Backchannel.SendAsync(request, Context.RequestAborted);
Expand All @@ -144,7 +212,6 @@ protected override async Task<OAuthTokenResponse> ExchangeCodeAsync(
}

var payload = JObject.Parse(await response.Content.ReadAsStringAsync());

return OAuthTokenResponse.Success(payload);
}
}
Expand Down
25 changes: 14 additions & 11 deletions src/AspNet.Security.OAuth.Shopify/ShopifyAuthenticationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ namespace AspNet.Security.OAuth.Shopify
/// <inheritdoc />
public class ShopifyAuthenticationOptions : OAuthOptions
{
public string ShopifyScope { get; set; } = "read_customers";

/// <summary>
/// An alias for ClientId
/// An alias for <see cref="OAuthOptions.ClientId"/>
/// </summary>
public string ApiKey
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need these aliases?

{
Expand All @@ -26,7 +24,7 @@ public string ApiKey
}

/// <summary>
/// An alias for ClientSecret
/// An alias for <see cref="OAuthOptions.ClientSecret"/>
/// </summary>
public string ApiSecretKey
{
Expand All @@ -45,13 +43,18 @@ public ShopifyAuthenticationOptions()
AuthorizationEndpoint = ShopifyAuthenticationDefaults.AuthorizationEndpoint;
TokenEndpoint = ShopifyAuthenticationDefaults.TokenEndpoint;
UserInformationEndpoint = ShopifyAuthenticationDefaults.UserInformationEndpoint;

Scope.Add(ShopifyScope);

// ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "id");
// ClaimActions.MapJsonKey(ClaimTypes.Name, "full_name");
ClaimActions.MapJsonSubKey(ClaimTypes.Dns, "shop", "myshopify_domain");
ClaimActions.MapJsonSubKey(ClaimTypes.NameIdentifier, "shop", "id", "long");

ClaimActions.MapJsonSubKey(ClaimTypes.NameIdentifier, "shop", "myshopify_domain");
ClaimActions.MapJsonSubKey(ClaimTypes.Name, "shop", "name");
ClaimActions.MapJsonSubKey(ClaimTypes.Webpage, "shop", "domain");
ClaimActions.MapJsonSubKey(ClaimTypes.Email, "shop", "email");
ClaimActions.MapJsonSubKey(ClaimTypes.Country, "shop", "country_code");
ClaimActions.MapJsonSubKey(ClaimTypes.StateOrProvince, "shop", "province_code");
ClaimActions.MapJsonSubKey(ClaimTypes.PostalCode, "shop", "zip");
ClaimActions.MapJsonSubKey(ClaimTypes.Locality, "shop", "primary_locale");
ClaimActions.MapJsonSubKey(ShopifyAuthenticationDefaults.ShopifyPlanNameClaimType, "shop", "plan_name");
ClaimActions.MapJsonSubKey(ShopifyAuthenticationDefaults.ShopifyEligableForPaymentsClaimType, "shop", "eligible_for_payments", "boolean");
ClaimActions.MapJsonSubKey(ShopifyAuthenticationDefaults.ShopifyTimezoneClaimType, "shop", "timezone");
}
}
}
Loading