magento / architecture

A place where Magento architectural discussions happen
275 stars 155 forks source link

Login as customer #414

Closed paliarush closed 4 years ago

paliarush commented 4 years ago

Requriements As a developer, I want to create a flexible login as a customer schema that can be added to the graph so a merchant or merchant admin can log into an end customer's account to assist them with their account.

Additional Changes

paliarush commented 4 years ago

Pending additional scenarios from @naydav

DrewML commented 4 years ago

Might want to add a note that this feature is only added to the B2B schema

paliarush commented 4 years ago

@DrewML it is mentioned that there is a dependency on LoginAsCustomer module. So obviously GraphQL module must be created in the same repository. I think it should be sufficient info on the location of the module.

DrewML commented 4 years ago

So obviously GraphQL module must be created in the same repository

Just pointing out, as a reviewer, that it wasn't clear to me this was a B2B only feature

because apparently it's not 😂

naydav commented 4 years ago

A few things from my side,

1) authentication_data_expiration_time is not related to the expiration time of the Magento backend/frontend session. This option is used only for internal module purposes related to redirect between Admin panel and Frontend (how long hash key for redirect fro Admin to Frontend is applicable). Thus from GraphqQL side, it doesn't make sense.

2) One of the most important things is logging. The current logs system is based on logic of controllers (CE/EE have different logging mechanisms). Thus from security side, it's so important to introduce new mechanism (maybe it will be required introducing some "hooks" in GraphQL processing in general) for listening to all of the requests.

3) About dependencies. In general, we consider that all of the external modules should depend only on LoginAsCustomerApi. The exception case is ACL definition which is located is in LoginAsCustomerAdminUi. We don't have a base approach for such cases but perhaps need to make ACL part of API and move to the appropriate module.

4) We had a few questions about integration with PWA, looks like for start will be enough: -) Call to REST endpoint for generation admin token. -) Call to GraphQL mutation with admin token from the previous step to generate "login as customer" token. -) Perform all of the needed GraphQL queries (log actions based on token)

5) In the scope of "Allow remote shopping assistance" the new customer option was introduced. Need to expand Customer type with a new option.

naydav commented 4 years ago

So obviously GraphQL module must be created in the same repository

Just pointing out, as a reviewer, that it wasn't clear to me this was a B2B only feature

@DrewML No, in general, it's CE feature. But EE contains custom logic for logging, B2B contains some additional buttons for functionality. https://docs.magento.com/user-guide/customers/login-as-customer.html?itm_source=merchdocs&itm_medium=quick_search&itm_campaign=federated_search&itm_term=login%20as%20cusome

DrewML commented 4 years ago

This feature has some security risks associated with it, that I think we should reconsider.

With the design of this feature, it seems like a headless UI (like pwa-studio) would use it in the following way:

  1. Store admin logs into back-office, finds customer, and clicks "Login as Customer"
  2. An admin token and the customer's email are passed to the storefront, which will be on a separate domain for headless/pwa (no session cookie will be present)
    • The only mechanism to pass this data is via a segment or querystring in a GET
  3. Headless/PWA reads the customer email and admin token from the URL, and calls GraphQL to get a token

My concern is around the second step here. We should never be passing sensitive information like an admin token in a URL, as it'll end up in web logs/referrer headers/etc.

Anyone that gets control of that admin token would then have the same level of access that the admin had, for as long as the session remains valid.

If we need to, from the admin, establish a session with that user in the headless application, there are patterns to do this that limit exposure to sensitive data/account takeover. One approach is:

  1. Store admin logs into back-office, finds customer, and clicks "Login as Customer"
  2. Admin UI/API generates a one-time use exchange token via some admin REST API endpoint
    • This token has an incredibly short TTL
    • This token is invalidated immediately upon use
    • This token only grants access for the target shopper's account
    • This token grants no other access
  3. Admin UI forwards admin user to headless storefront, with one-time exchange token in URL
  4. Headless storefront calls GraphQL API, and exchanges one-time exchange token for a customer token
DrewML commented 4 years ago

No, in general, it's CE feature

Interesting! I was going off the project overview from the Product Team in Confluence:

We believe that introducing the capability sales Rep to login as customer, for B2B Sales Representative , we will satisfy a basic need during and after the B2B shopping experience

There's also the note from @mbrinton01 in B2B-221:

This feature does apply across B2B and B2C. It should be present in Commerce but does not need to be in Open Source.

The devdocs linked also have "Login as Customer logging" as a Commerce only feature, but seems like logging was implemented in Open-Source.

Can you clarify if we changed this requirement somewhere? Just very confused 😅

nathanjosiah commented 4 years ago

From the security team's perspective, I agree with @DrewML here. When it comes to authentication workflows, there is no need to completely reinvent our own mechanism when there are many well-documented and reviewed workflows.

On that note, the workflow that @DrewML already described is very similar to how one of the oauth workflows function. I would recommend to stick as closely to the oauth handshake as possible since it addresses all the concerns listed here. It isn't 100% identical but the relevant portion can be used. Of course, the full oauth workflow described in the rfc is a little more than needed here because we aren't dealing with multiple apps and different scopes and permission types.

However, the heart of the workflow is definitely still relevant.

That said, here is what it would roughly look like using the oauth 2 authorization code workflow rfc 6749 section 1.3.1.

  1. "Super" Admin defines a storefront callback URL.
  2. Some admin clicks login as customer
  3. Magento creates and stores an authorization code and associates it to the customer being requested.
  4. The admin is redirected to the defined storefront url from step 1 with the created code.
  5. The storefront sends the authorization token via XHR POST request (in the body)
  6. Magento responds with a valid admin/integration token for the storefront to make authenticated requests with.

⚠ī¸ Now, it's important to note that no matter what we do with this workflow there will always be a weak point of the magento redirect containing everything needed to get a token. Oauth elminates this issue by using a shared secret between the client and the server (called the client secret). This is not possible because a PWA has no state and no way to have a shared secret unique to each user.

💡 We can mitigate this small risk by

  1. Ensure the authorization code is only valid once
  2. Ensure the authorization code is only valid for a short period of time

ℹī¸ We could potentially use the admin username/password in the request as the shared secret but I don't think it would be ideal from a UI/UX view to prompt the admin again for their credentials on the storefront although it would eliminate the issue.

mbrinton01 commented 4 years ago

The bullet point quoted above is the answer to a question about admin permissions, not the overall feature. This feature is available in both Open Source and Commerce.

Mark

From: Andrew Levine notifications@github.com Reply-To: magento/architecture reply@reply.github.com Date: Monday, August 17, 2020 at 4:24 PM To: magento/architecture architecture@noreply.github.com Cc: Mark Brinton mbrinton@adobe.com, Mention mention@noreply.github.com Subject: Re: [magento/architecture] Login as customer (#414)

No, in general, it's CE feature

I'm going off project overview from the Product Team in Confluence:

We believe that introducing the capability sales Rep to login as customer, for B2B Sales Representative , we will satisfy a basic need during and after the B2B shopping experience . We will know this to be true when we see merchants using this feature actively.

There's also the note from @mbrinton01https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmbrinton01&data=02%7C01%7Cmbrinton%40adobe.com%7Caeb74127ab484fae5fd508d842f3dd77%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637332962448761639&sdata=rLNBYZ5UrEFLGaGGU8t9UD8PQIBFr2a2GArL4TB3noE%3D&reserved=0 in B2B-221:

This feature does apply across B2B and B2C. It should be present in Commerce but does not need to be in Open Source.

Can you clarify if we changed this requirement somewhere?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmagento%2Farchitecture%2Fpull%2F414%23issuecomment-675123051&data=02%7C01%7Cmbrinton%40adobe.com%7Caeb74127ab484fae5fd508d842f3dd77%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637332962448771632&sdata=TD2%2BV%2FGEkuS7VKdxLvBUBhEVJ7Y9N0wllYNIGhz2exw%3D&reserved=0, or unsubscribehttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FACRESEBA3XMWZTJEAHYWBOLSBGNXHANCNFSM4PUUAGKA&data=02%7C01%7Cmbrinton%40adobe.com%7Caeb74127ab484fae5fd508d842f3dd77%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637332962448771632&sdata=PBZOVm7vzeMGRXXaJjNP%2FNi0fpxCkhhvw8b2EzOBifU%3D&reserved=0.

AlexMaxHorkun commented 4 years ago

I would suggest trying a more simple approach that would work for REST as well - create a REST endpoint that would generate customer token for an admin, then use the token same as you would've used a regular customer token for REST and GraphQL web APIs

DrewML commented 4 years ago

I would suggest trying a more simple approach that would work for REST as well - create a REST endpoint that would generate customer token for an admin, then use the token same as you would've used a regular customer token for REST and GraphQL web APIs

This poses the same level of risk - getting that admin-generated token to the headless front-end (from the admin UI) would require passing it via the URL in a GET (browser navigation). Any token passed via URL should be short lived and have minimal permissions

nathanjosiah commented 4 years ago

REST/SOAP/GraphQL it doesn't matter. There will need to a new way to exchange an authorization code (from the described workflow) for an admin token. It doesn't eliminate the need for the authorization code or the exchange.

AlexMaxHorkun commented 4 years ago

@paliarush I can't figure out the reason why headless Magento clients would have to pass customer token issue for an admin via URL, can you help?