Refactoring entry-point for Authentication Flow

[dbnoble]

I want to get your thoughts on this before spending the time to make the modifications for a pull request.

Currently, the only in-package way to begin the authorization flow without any "bespoke" code is using openAuthTab, and the post_auth_uri logic that is used is locked up in the Authenticator.

I started on a pull request to add the following code to Authenticators.js which I am using currently.

// Navigate directly to auth_uri in current tab
// You MUST provide a post_auth_uri for this to work properly
Shopify.PublicAppOAuthAuthenticator.prototype.goToAuth = function() {
    if (!Meteor.isClient) {
        throw new Error("Can only open a new tab client-side.");
    }
    window.top.location.href = this.auth_uri;
};

However, the "gotcha" of this breaking if you don't specify a post_auth_uri does not sit will with me for public usage, and I quickly found that it is not possible to overwrite this.config.post_auth_uri from here since the authConfig has already been passed in to the handler via a method call.

What I would like to propose is a single function to handle entry to authentication flow that relies on an authType passed to the Authenticator options with an appropriate fallback/default for the post_auth_uri for each authType inside Shopify.PublicAppOAuthAuthenticator.

The current functionality contained within openAuthTab would be handled with authType: 'popup', direct navigation would be authType: 'redirect', and the auth flow could be started with authenticator.startAuth();

Shopify.PublicAppOAuthAuthenticator.prototype.startAuth() = function() {
  if (!Meteor.isClient) {
    throw new Error("Authentication flow is client-side only");
  }
  if(this.authType == 'redirect') {
    window.top.location.href = this.auth_uri;
  } else if (this.authType == 'popup') {
    window.open(this.auth_uri, "_blank");
  }
};

Then in Shopify.PublicAppOAuthAuthenticator something like this:

if (this.authType == 'popup') {
  //use specified URI or close tab
  this.config.post_auth_uri = options.post_auth_uri || "close";
} else {
  //All other uses, use specified URI or redirect to original location
  this.config.post_auth_uri = options.post_auth_uri || window.top.location.href;
}

If you are open to this, I will move ahead and make a pull request, and follow up with another for a couple other things I would like to see in the interest of optimizing for embedded app usage, such as an option to pass something like "embed" to post_auth_uri so that the path to the embedded app can be formed inside of PublicAppOAuthAuthenticator instead of inline since the shop and api key are already available there.

[froatsnook]

First of all, thank you. The auth is definitely the most strange/confusing part of this package, so any improvements sound great to me.

I think that your pull request sounds totally sensible. Using "close" as the default post_auth_uri only makes sense when the user is using openAuthTab. In the README I talk about two other scenarios (iframe and direct navigation). A good default post_auth_uri in these scenarios would be good.

So help me understand your auth workflow. I take it from your comments that you're using an embedded app. Does the auth process start when the user navigates to your webpage directly and you say "login via shopify"? Or is your app in an iframe within the shopify admin page, then changing window.top.location.href navigates to shopify login? In this case, redirecting to the original page as you've done makes sense.

As for the pull request, make sure that openAuthTab sticks around in order to avoid breaking backward compatibility. Possibly it can just set authType to popup and call startAuth.

What do you think of the following API?

authenticator.openAuthTab(); // as it is now
authenticator.startAuth(); // directly navigates, as you implemented it
authenticator.startAuthInIframe(iframe); // navigates an iframe for auth

Then the user wouldn't specify which authType they are using. Rather it would be decided when they called one of the three auth methods. And the post_auth_uri has a default value in each case ("close" for openAuthTab, window.top.location.href for startAuth and iframe.location.href for startAuthInIFrame).

[dbnoble]

I am only using window.top.location.href as an example because it's the only thing I can think of that makes sense as a default since we aren't psychic. In other words, it's better for the default to go SOMEWHERE instead of just going to a white screen with no further information if they forget to set a post_auth_uri when using direct navigation.

I am using an embedded app, but that should be taken into consideration only in that I am including it as one of the potential use cases that we can optimize for. My intent is to do what makes sense for the most potential use cases with the least amount of code.

That being said, I know that you have mentioned previously that you have not worked much with embedded apps so I will go over some of the pertinent details here.

You cannot access an embedded app at all unless you have already gone through the auth flow to begin with. Once you have gone through the auth flow, the link to the app from the Shopify dashboard links directly to the embed. In other words, the initial authentication required to add an app to your shop never originates from within the embedded app iframe.

I can think of only 2 cases where the user would need to begin the auth flow from within the iframe embedded app itself:

1. You are using accounts/storing credentials and need to get permissions for a different set of scopes than what you initially requested.
2. You are not using accounts/storing credentials and therefore must reauthenticate on every visit.

Scenario 1. I would consider more of an edge case where it would make sense to handle that with custom code that probably does not need to be in-package.

Scenario 2. Could be handled by whichever in-package authType you wish, but would also require some type of logic that checks for an existing keyset and starts the authentication flow if none is found.

When a user adds an embedded app via the App Store, they will either be directed to a landing page which you specified OR simply go DIRECTLY to the auth flow that adds the app. Since we currently MUST have a handler with this package (which in all honesty is probably not ideal), if we are not using accounts, we will always need to send them to a page that, at the very least, sets up the authenticator and immediately redirects to the auth_uri. In that case, direct navigation would be necessary, and you would set the post_auth_uri to redirect back to the embedded app.

Hopefully I have made all of that clear, but if you have any questions regarding that part of the flow, please let me know.

Now, moving back to working with an authType approach. The important thing here is that we cannot define 'post_auth_uri' logic in a separate function like authenticator.openAuthTab() or authenticator.startAuth() because the authConfig is already sent to the handler when the authenticator is defined. That is the entire reason I propose moving to an authType that is handled inside the authenticator. authenticator.startAuth() would simply initiate the authentication flow using the authType that we specified when defining the authenticator.

Unfortunately I'm short on time so I have to run for the moment, but if you have any questions or thoughts thus far, please let me know and I will follow-up shortly.

[dbnoble]

Alright, so let's look at some common usage scenarios where different authTypes would be used. This should also highlight why passing a string like "embed" to post_auth_uri would be useful by using the authenticator.config to constuct the embedded app URI similar to the way auth_uri is constructed currently.

Embedded Apps

Initial Authentication authType: "redirect", post_auth_uri: "embed"

• Landing Page -> Shopify Auth -> Embedded app
• App Store -> Gateway to set authenticator & handler -> Shopify Auth -> Embedded app
• Meteor.user protected area -> Shopify Auth -> Embedded app

Reauthentication authType: any, post_auth_uri: "embed"

• Embedded app -> Shopify Auth -> Embedded App

Traditional Apps

Initial Authentication authType: any, post_auth_uri: specify or default

• Landing Page -> Shopify Auth -> ???
• App Store -> Shopify Auth -> ???
• Meteor.user protected area -> Shopify Auth -> ???

Reauthentication authType: any, post_auth_uri: specify or default

• App -> Shopify Auth -> ???

Default post_auth_uri for authTypes:

• popup: "close"
• redirect: window.top.location.href
• iframe: n/a

If I have an embedded app, chances are all the functionality is held within the embedded app itself, and it doesn't make sense to use anything other than direct
navigation and route back to the embedded app after auth (although a new window that redirects instead of closing is also a sensible way to begin the Auth flow from a landing page or member's backend). This is what led me to the conclusion that I that there should be a direct navigation equivalent of openAuthTab() so that we have something in-package that we can easily document and provide demo code for.

window.top.location.href as a default will allow the "redirect" method to work for for both embedded apps and traditional apps as required, and specify the post_auth_uri when necessary for the functionality you need.

We could also check for for embedded_app_sdk and use the constructed embedded_app_uri as the default if that is preferable.

If we do move to an authType method inside the authenticator, I definitely agree that openAuthTab should be left in place for backwards compatibility, although potentially considered deprecated with an appropriate deprecation notice.

I should also note that I don't see enough of a use case for an iframe authType, but I would be happy to include it if you think it is necessary enough to have an in-package method.

[dbnoble]

Another possibility would be to move the method call that passes the authConfig to a handler into the proposed startAuth() method so that the post_auth_uri can still be modified after the authenticator is defined. We could then decide if it makes more sense to use an authType method inside the authenticator options object as proposed OR to simply pass an authType directly: authenticator.startAuth('popup');

Sorry I know this is all very long winded and might be hard to visualize. I keep finding myself wanting to explain things in JSON format to organize the thought process, but that would just be ridiculous! :)

[froatsnook]

OK I'm starting to wrap my head around this.

I like the following, but it's backwards incompatible. See below.
First of all, I think it would make sense to move the handler installation to startAuth so that authType can be chosen at startAuth-time. The only potential issue I see is a race condition... what if the user calls startAuth("popup"), approves the app, and Shopify calls the "__shopify-auth" link all before the handler gets installed server-side. Sounds unlikely, but something to think about. At the moment the meteor method doesn't return until the entire auth has happened. The only "safe" solution would be to refactor AuthRedirect.js a bit and then use two meteor methods (one for installing the handler, another which waits for it to be called).

If you move the current Meteor.call("froatsnook:shopify/HandleAuthRedirect", ..., then I can implement this refactor, so don't worry about it if the above sounds annoying!

Next, using embedded_app_uri sounds sensible. How is that generated?

I just glanced through the code. The embedded_app_sdk parameter is only used in AuthRedirect.js like this:

        if (authenticatorConfig.post_auth_uri === "close") {
            res.writeHead(200, { });
            res.end("<html><head><script>window.close()</script></head><body></body></html>");
        } else if (authenticatorConfig.embedded_app_sdk) {
            res.writeHead(200, { });
            res.end("<script type='text/javascript'>window.top.location.href = '" + authenticatorConfig.post_auth_uri + "';</script>");
        } else {
            res.writeHead(302, {
                "Location": authenticatorConfig.post_auth_uri
            });
            res.end();
        }

I'm not sure why I did it like that. In the EmbeddedAppSDKTest I set embedded_app_sdk: true in the authentciator but then I leave the post_auth_uri as "close" and call openAuthTab (in other words, setting embedded_app_sdk: true in the example does literally nothing). It looks like the server expects the client to be running in an iframe and wants to redirect the top window to post_auth_uri. I'll investigate this more.

If we do move to an authType method inside the authenticator, I definitely agree that openAuthTab should be left in place for backwards compatibility, although potentially considered deprecated with an appropriate deprecation notice.

While startAuth is definitely the cleaner API, I'd like to leave openAuthTab in, then deprecate for 2.0, and remove in 3.0. I don't want to give people deprecation warnings for non-major version changes.

I should also note that I don't see enough of a use case for an iframe authType, but I would be happy to include it if you think it is necessary enough to have an in-package method.

I agree that it doesn't seem that important to be in-package. But this made me realize that doing the handler installation in startAuth would be backwards incompatible! The README's iframe example (as well as the direct navigation example) won't work anymore if we move the handler installation to startAuth.

So an alternate proposal: leave the Meteor.call where it is in the authenticator constructor. And then when the user calls startAuth, do another Meteor.call to update the authenticator based on authType. It sounds like a bit of a hack, but I really want to be able to choose authType in startAuth, but I also want to make sure it's backwards compatible.

Thanks for the ideas and input!

[froatsnook]

As for default post_auth_uri, I agree about using "close", embedded_app_uri and window.top.location.href. These sound sensible.

[froatsnook]

OK I see that I added that code for a reason.

From Shopify's docs:

OAuth will behave normally in your app, as it would for any other Shopify apps or other OAuth enabled services with one caveat:

Since the application is loaded inside an iframe it is critical that the initial OAuth request redirect escapes the iframe to make the requests. Shopify returns the X-Frame-Options=DENY header and prevents any Shopify admin pages from being loaded inside an iframe.

This means that where the OAuth process would normally begin with:

redirect_to "/auth/shopify?shop=myshopname"

It should now return a page containing:

<script type='text/javascript'>
  window.top.location.href = '/auth/shopify?shop=myshopname';
</script>

And then there's a reminder that says:

The above example assumes you're using the Shopify Omniauth gem. If not, replace /auth/shopify with the supported http://myshop.com/admin/oauth endpoint.

So I'm guessing that we could come up with a good default post_auth_uri for embedded app sdk apps, but I'm not sure what it is. Is this the embedded_app_uri you've been mentioning?

[dbnoble]

To construct embedded_app_uri it would be 'https://' + this.config.shop + '.myshopify.com/admin/apps/' + this.config.api_key

I think it would be best to still allow an override via a specified post_auth_uri but it is definitely a reasonable default for embedded apps.

[dbnoble]

I would also like to see a way to specify a default handler config for requests that originate from the App Store so that we don't have to rely on a gateway page to define the authenticator and set a handler.

I'll have to look into how to accomplish that.

[froatsnook]

I would also like to see a way to specify a default handler config for requests that originate from the App Store so that we don't have to rely on a gateway page to define the authenticator and set a handler.

I 100% agree with this one. My idea is to:

1. Call installWebAppHandler on server startup
2. Encourage (via README) all users who want to authenticate in this way (i.e. everyone who's not just making a shopify "integration") to call server-side Meteor.onAuth.
3. In this case we want to redirect to post_auth_uri since the user is in an iframe

The issue is we need to identify the user client-side after the redirect. My best idea on this one would be to pass on the query parameters that shopify gives to __shopify-auth to post_auth_uri, i.e. we'll redirect to ${post_auth_uri}?shop=ABC&hmac=DEF&.... Then client-side the user could call something like Shopify.verifyAuth(this.params, keyset, function(err, shop) { ... }) which would
use the shared secret in keyset to check the signature in the query params to make sure that the user is really authenticated at shop.

I'll think about this more.

[dbnoble]

Actually a gateway page isn't a huge deal if we can set the default handler using webapphandler instead of initiating it the normal way, that would solve the problem with the UUID not being set for the state parameter.

That should be doable without actually having to load all the Meteor clientside JS, right? I just hate having to load up the entire Meteor client bundle for something a simple as a redirect which is the only thing I have against the gateway page.

I think that should work, but I'll let you figure that one out, I have no experience with webbapphandler.

I've basically already done this with a serverside route in iron:router and it would be fast and easy, but it's not good solution for most users and I'd like the package to continue to be router agnostic.

I will begin working on the pull request for the other things once I am finished with the project I am finishing up and draw out a spec for the changes.