-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds Multipass functionality #65
Conversation
Nice, thank you. Will have a closer look tomorrow :) |
def get_url(shop_name, customer_data, multipass_secret, options \\ %{}) do | ||
struct( | ||
%URI{ | ||
host: "#{shop_name}.myshopify.com", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When asking for the shop name, it is often unclear which version one should use: myshop.myshopify.com
or just myshop
. For that reason, there is a scrub_shopify_name
function, that will return the shop name without myshopify.com
. Since it's already there, we can use it here
# ...
host: Shopify.scrub_shop_name(shop_name) <> ".myshopify.com"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that problematic for stores that use fully custom domain names? E.g. we have a production site that uses somesite.com
as its Shopify domain. So I think requests have to be made against that domain, and .myshopify.com
must be omitted entirely (pending production testing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shops I have worked with that use custom domains all still have their "original" shopify domain, and the admin page is not even redirected, so the API works w/o problems. This is my personal experience, so there is room for some error 🙇
I haven't really worked with any of the Gold Features, so this might actually be one of the cases, where you would want the custom URL, not the original one, to avoid an unnecessary redirect.
If that's the case, I'd suggest using the "raw input" for the host, and put an emphasis on having to use "my-shop.myshopify.com" or the custom domain in the Readme. 🤔
Returns the encrypted message as cipher text. | ||
""" | ||
@spec encrypt(binary, binary, integer) :: binary | ||
def encrypt(message, encryption_key, block_size \\ @block_size) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to make the token testable, we could move the encryption part into a separate module, define a behaviour and implement a test version. That way everyone can either roll with the provided mock encryption (for tests), or implement their own - pretty much how the http adapter works.
I am fine with doing that later, or doing it myself and getting this merged as it is though 👍
@fireproofsocks - is there any purpose to exposing the internal functions used my |
The internal functions arguably could/should be private via |
Alternatively, we could mark them as internal with |
This implements #64
The Multipass is available to Shopify Plus plans. It allows your non-Shopify site to be the source of truth for authentication and login. After your site has successfully authenticated a user, redirect their browser to Shopify using the special Multipass URL: this will upsert the customer data in Shopify and log them in.
Unlike other API requests, this does not require a session: it relies on a shared secret to do decryption.
Your customer data must at a minimum provide an email address and a current datetime in 8601 format.