Skip to content

LoginAsCustomer GraphQL #30130

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

Merged
merged 21 commits into from
Dec 4, 2020
Merged
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\LoginAsCustomerGraphQl\Model\LoginAsCustomer;

use Magento\Customer\Model\Customer;
use Magento\Framework\Exception\LocalizedException;
use Magento\Integration\Model\Oauth\TokenFactory;
use Magento\Store\Model\StoreManagerInterface;
use Magento\Framework\GraphQl\Exception\GraphQlInputException;

/**
* Create customer token from customer email
*
* Class CreateCustomerToken
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, remove the unnecessary comments. These comments are auto-generated and the only purpose is to add extra lines to your code.

Suggested change
* Class CreateCustomerToken

Choose a reason for hiding this comment

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

Removed

*/
class CreateCustomerToken
{
/**
* @var Customer
*/
private $customer;

/**
* @var TokenFactory
*/
private $tokenModelFactory;

/**
* @var StoreManagerInterface
*/
private $storeManager;

/**
* CreateCustomerToken constructor.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, remove unecessary comments

Suggested change
* CreateCustomerToken constructor.

Choose a reason for hiding this comment

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

Removed

* @param StoreManagerInterface $storeManager
* @param TokenFactory $tokenModelFactory
* @param Customer $customer
*/
public function __construct(
StoreManagerInterface $storeManager,
TokenFactory $tokenModelFactory,
Customer $customer
) {
$this->storeManager = $storeManager;
$this->tokenModelFactory = $tokenModelFactory;
$this->customer= $customer;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, do not inject models using DI. Use customerFactory to create the corresponding instance instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

In case if you need to work with the existing customer instance, you may pass it as an argument to the execute method.

}

/**
* @param string $email
* @return array
* @throws LocalizedException
*/
public function execute(string $email)
{
$websiteID = $this->storeManager->getStore()->getWebsiteId();
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not recommend using this approach since it might be inconsistent (there's a chance of getting wrong store ID). In GraphQL area we get information about the current store from the GraphQL execution context but not from the store manager.

Please, take a look at the following example

$storeId = (int)$context->getExtensionAttributes()->getStore()->getId();

That's the correct and recommended way of getting the current store. You may pass the website id as an argument to this function from resolver.

$this->customer->setWebsiteId($websiteID)->loadByEmail($email);

/* Check if customer email exist */
if (!$this->customer->getId()) {
throw new GraphQlInputException(
__('Customer email provided does not exist')
);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We also need to validate that the 'allow_remote_shopping_assistance' setting is enabled for the customer. See https://github.com/magento/architecture/pull/414/files#diff-ccdc60253a5ea983cb68248c51500c3cdc077232bd52ded6cb08d4af9cba8d2bR39

Copy link

Choose a reason for hiding this comment

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

Done

$customerId = $this->customer->getId();
$customerToken = $this->tokenModelFactory->create();
return [
"customer_token" => $customerToken->createCustomerToken($customerId)->getToken()
Copy link
Contributor

Choose a reason for hiding this comment

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

We may have an unhandled exception upon creating customer token. For example if the system is unable to save a token to the database. I would recommend catching exception here.

];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\LoginAsCustomerGraphQl\Model\Resolver;

use Magento\Framework\AuthorizationInterface;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\GraphQl\Config\Element\Field;
use Magento\Framework\GraphQl\Exception\GraphQlAuthorizationException;
use Magento\Framework\GraphQl\Exception\GraphQlNoSuchEntityException;
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;
use Magento\Framework\GraphQl\Query\Resolver\Value;
use Magento\Framework\GraphQl\Query\ResolverInterface;
use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
use Magento\LoginAsCustomerApi\Api\ConfigInterface;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend using aliases for such imports. Just to make it more clear what config interface your code is about.

Suggested change
use Magento\LoginAsCustomerApi\Api\ConfigInterface;
use Magento\LoginAsCustomerApi\Api\ConfigInterface as LoginAsCustomerConfig;

use Magento\LoginAsCustomerGraphQl\Model\LoginAsCustomer\CreateCustomerToken;

/**
* Gets customer token
*
* Class RequestCustomerToken
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, omit the class name in the DocBlock, but leave the short description.

*/
class RequestCustomerToken implements ResolverInterface
{
/**
* @var ConfigInterface
*/
private $config;

/**
* @var AuthorizationInterface
*/
private $authorization;

/**
* @var CreateCustomerToken
*/
private $createCustomerToken;

/**
* RequestCustomerToken constructor.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, omit a short description of constructors. They don't provide useful information but use one more line of the file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please, remove unnecessary comments

* @param AuthorizationInterface $authorization
* @param ConfigInterface $config
* @param CreateCustomerToken $createCustomerToken
*/
public function __construct(
AuthorizationInterface $authorization,
ConfigInterface $config,
CreateCustomerToken $createCustomerToken
) {
$this->authorization = $authorization;
$this->config = $config;
$this->createCustomerToken = $createCustomerToken;
}

/**
* Get Customer Token using email
*
* @param Field $field
* @param ContextInterface $context
* @param ResolveInfo $info
* @param array|null $value
* @param array|null $args
* @return Value|mixed|void
* @throws GraphQlAuthorizationException|GraphQlNoSuchEntityException|LocalizedException
*/
public function resolve(
Field $field,
$context,
ResolveInfo $info,
array $value = null,
array $args = null
) {
$isAllowedLogin = $this->authorization->isAllowed('Magento_LoginAsCustomer::login');
$isEnabled = $this->config->isEnabled();

/* Get input params */
try {
$args = $args['input'] ?: [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, correct me if I'm wrong, but there's no chance of getting NoSuchEntityException from this expression. Instead of try->catch we may use a simple condition and throw GraphQlInputException in case of missing argument.

} catch (NoSuchEntityException $e) {
throw new GraphQlNoSuchEntityException(__('Check input params.'));
}

if (empty(trim($args['customer_email'], " "))) {
throw new GraphQlInputException(__('Specify the "customer email" value.'));
}

$this->validateUser($context);

if (!$isAllowedLogin || !$isEnabled) {
throw new GraphQlAuthorizationException(
__('Login as Customer is disabled.')
);
}

return $this->createCustomerToken->execute($args['customer_email']);
}

/**
* Check if its an admin user
*
* @param $context
* @throws GraphQlAuthorizationException
*/
private function validateUser($context)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, use type hinting for method parameters as well as for the return types

{
if ($context->getUserType() != 2 || $context->getUserId() == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, use strict comparison

Suggested change
if ($context->getUserType() != 2 || $context->getUserId() == 0) {
if ($context->getUserType() !== 2 || $context->getUserId() === 0) {

throw new GraphQlAuthorizationException(__('The current customer isn\'t authorized.'));
}
}
}
51 changes: 51 additions & 0 deletions app/code/Magento/LoginAsCustomerGraphQl/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Module Magento LoginAsCustomerGraphQl

Copy link
Contributor

Choose a reason for hiding this comment

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

Is all of the info in this README relevant for this module?

Choose a reason for hiding this comment

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

@keharper I have formatted readme file with the relevant content, Thanks

``magento/module-loginascustomergraphql``

- [Main Functionalities](#markdown-header-main-functionalities)
- [Installation](#markdown-header-installation)
- [Configuration](#markdown-header-configuration)
- [Specifications](#markdown-header-specifications)
- [Attributes](#markdown-header-attributes)


## Main Functionalities
Flexible login as a customer so a merchant or merchant admin can log into an end customer's account to assist them with their account.

## Installation
\* = in production please use the `--keep-generated` option

### Type 1: Zip file

- Unzip the zip file in `app/code/Magento`
- Enable the module by running `php bin/magento module:enable Magento_LoginAsCustomerGraphQl`
- Apply database updates by running `php bin/magento setup:upgrade`\*
- Flush the cache by running `php bin/magento cache:flush`

### Type 2: Composer

- Make the module available in a composer repository for example:
- private repository `repo.magento.com`
- public repository `packagist.org`
- public github repository as vcs
- Add the composer repository to the configuration by running `composer config repositories.repo.magento.com composer https://repo.magento.com/`
- Install the module composer by running `composer require magento/module-loginascustomergraphql`
- enable the module by running `php bin/magento module:enable Magento_LoginAsCustomerGraphQl`
- apply database updates by running `php bin/magento setup:upgrade`\*
- Flush the cache by running `php bin/magento cache:flush`


## Configuration




## Specifications




## Attributes



26 changes: 26 additions & 0 deletions app/code/Magento/LoginAsCustomerGraphQl/composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"name": "magento/module-loginascustomergraphql",
"description": "Flexible login as a customer so a merchant or merchant admin can log into an end customer's account to assist them with their account.",
"type": "magento2-module",
"require": {},
"version": "1.0.0",
"require": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, provide all the dependencies you use within the scope of the module. Like LoginAsCustomerApi, Customer, Store etc

"php": "~7.1.3||~7.2.0||~7.3.0",
"magento/framework": "102.0.*"
},
"suggest": {
"magento/module-graph-ql": "100.3.*"
},
"license": [
"OSL-3.0",
"AFL-3.0"
],
"autoload": {
"files": [
"registration.php"
],
"psr-4": {
"Magento\\LoginAsCustomerGraphQl\\": ""
}
}
}
11 changes: 11 additions & 0 deletions app/code/Magento/LoginAsCustomerGraphQl/etc/module.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" ?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_LoginAsCustomerGraphQl" setup_version="1.0.0">
Copy link
Contributor

Choose a reason for hiding this comment

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

You may omit the setup_version since we don't introduce changes to the database. In case it will happen in the future, it's recommended to use declarative schema that does not require setup_version either.

<sequence>
<module name="Magento_Customer"/>
<module name="Magento_LoginAsCustomer"/>
<module name="Magento_Authorization"/>
<module name="Magento_GraphQl"/>
</sequence>
</module>
</config>
27 changes: 27 additions & 0 deletions app/code/Magento/LoginAsCustomerGraphQl/etc/schema.graphqls
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
type Mutation {
generateCustomerTokenAsAdmin(
input: GenerateCustomerTokenAsAdminInput!
): GenerateCustomerTokenAsAdminOutput
@resolver(class: "Magento\\LoginAsCustomerGraphQl\\Model\\Resolver\\RequestCustomerToken")
@doc(description: "Request a token using Customer email")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@doc(description: "Request a token using Customer email")
@doc(description: "Request a customer token so that an administrator can perform remote shopping assistance")

Choose a reason for hiding this comment

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

Done

}

input GenerateCustomerTokenAsAdminInput {
customer_email: String!
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
customer_email: String!
customer_email: String! @doc(description: "The email address of the customer requesting remote shopping assistance")

Choose a reason for hiding this comment

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

Done

}

type GenerateCustomerTokenAsAdminOutput {
customer_token: String!
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
customer_token: String!
customer_token: String! @doc(description: "The generated customer token")

Choose a reason for hiding this comment

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

Done

}

type Customer {
allow_remote_shopping_assistance: Boolean!
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
allow_remote_shopping_assistance: Boolean!
allow_remote_shopping_assistance: Boolean! @doc(description: "Indicates whether the customer has enabled remote shopping assistance")

Choose a reason for hiding this comment

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

Done

}

input CustomerCreateInput {
allow_remote_shopping_assistance: Boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
allow_remote_shopping_assistance: Boolean
allow_remote_shopping_assistance: Boolean @doc(description: "Indicates whether the customer has enabled remote shopping assistance")

Choose a reason for hiding this comment

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

Done

}

input CustomerUpdateInput {
allow_remote_shopping_assistance: Boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
allow_remote_shopping_assistance: Boolean
allow_remote_shopping_assistance: Boolean @doc(description: "Indicates whether the customer has enabled remote shopping assistance")

Choose a reason for hiding this comment

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

Done

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Resolvers using these inputs will need to be updated to ensure allow_remote_shopping_assistance is persisted.

Copy link

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@cspruiell cspruiell Dec 2, 2020

Choose a reason for hiding this comment

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

I don't see the changes for this. The createCustomer & updateCustomer mutations need to be updated so that allow_remote_shopping_assistance is persisted

mutation { createCustomerV2(input: { email: "test@example.com" firstname: "Test" lastname: "Magento" password: "T3stP4ssw0rd" allow_remote_shopping_assistance: true}) { customer { firstname lastname email allow_remote_shopping_assistance } } }

4 changes: 4 additions & 0 deletions app/code/Magento/LoginAsCustomerGraphQl/registration.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?php
use Magento\Framework\Component\ComponentRegistrar;

ComponentRegistrar::register(ComponentRegistrar::MODULE, 'Magento_LoginAsCustomerGraphQl', __DIR__);
Loading