認可リクエスト画面の改善

EC-CUBE · Aug 3, 2020 · 9dfc9ff · 9dfc9ff
1 parent 4f186f4
commit 9dfc9ff
Show file tree

Hide file tree

Showing 6 changed files with 121 additions and 20 deletions.
diff --git a/EventListener/AuthorizationRequestResolveListener.php b/EventListener/AuthorizationRequestResolveListener.php
@@ -89,15 +89,17 @@ public function onAuthorizationRequestResolve(AuthorizationRequestResolveEvent $
             $content = $this->twig->render(
                 '@Api/admin/OAuth/authorization.twig',
                 [
-                    'scope' => join(' ', $event->getScopes()),
+                    'scopes' => $event->getScopes(),
                     'form' => $form->createView(),
                 ]
             );
 
             if ('POST' === $request->getMethod()) {
                 $form->handleRequest($request);
                 if ($form->isSubmitted() && $form->isValid()) {
-                    $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED);
+                    if ($form->get('approve')->isClicked()) {
+                        $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_APPROVED);
+                    }
                 } else {
                     $event->resolveAuthorization(AuthorizationRequestResolveEvent::AUTHORIZATION_DENIED);
                 }

diff --git a/Form/Type/Admin/OAuth2AuthorizationType.php b/Form/Type/Admin/OAuth2AuthorizationType.php
@@ -16,6 +16,7 @@
 use Eccube\Common\EccubeConfig;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\HiddenType;
+use Symfony\Component\Form\Extension\Core\Type\SubmitType;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\Validator\Constraints as Assert;
 
@@ -67,7 +68,9 @@ public function buildForm(FormBuilderInterface $builder, array $options)
                 'constraints' => [
                     new Assert\NotBlank(),
                 ],
-            ]);
+            ])
+            ->add('approve', SubmitType::class)
+            ->add('deny', SubmitType::class);
     }
 
     public function getBlockPrefix()

diff --git a/Resource/locale/messages.en.yaml b/Resource/locale/messages.en.yaml
@@ -17,6 +17,10 @@ api:
       delete__confirm_title: Delete a Client
       delete__confirm_message: Are you sure to delete this Client?
       clear_expired_tokens: Clears all expired access and/or refresh tokens
+      allow__confirm_message: Do you want to allow access this app to %shop_name%?
+      allow__confirm_description: 'Allow this app to:'
+      allow: Allow
+      deny: Deny
     webhook:
       management:  WebHook
       registration: WebHook Registration

diff --git a/Resource/locale/messages.ja.yaml b/Resource/locale/messages.ja.yaml
@@ -11,12 +11,18 @@ api:
       identifier: クライアントID
       secret: クライアントシークレット
       scope: スコープ
+      scope.read.description: %shop_name%のデータに対する読み取り
+      scope.write.description: %shop_name%のデータに対する書き込み
       redirect_uri: リダイレクトURI
       grant_type: グラントタイプ
       client_registration: OAuthクライアント登録
       delete__confirm_title: OAuthクライアントを削除します。
       delete__confirm_message: OAuthクライアントを削除してよろしいですか？
       clear_expired_tokens: 期限切れのアクセストークンとリフレッシュトークンを削除する
+      allow__confirm_message: アプリから%shop_name%へのアクセスを許可しますか？
+      allow__confirm_description: 'このアプリに以下を許可します:'
+      allow: 許可する
+      deny: 許可しない
     webhook:
       management:  WebHook管理
       registration: WebHook登録

diff --git a/Resource/template/admin/OAuth/authorization.twig b/Resource/template/admin/OAuth/authorization.twig
@@ -1,22 +1,42 @@
 {% extends '@admin/login_frame.twig' %}
 
+{% form_theme form '@admin/Form/bootstrap_4_horizontal_layout.html.twig' %}
+
 {% block main %}
-    <p>このアプリ連携を許可しますか？</p>
-    <p>このアプリは以下のアクセス権を要求しています:</p>
-    <p>{{ scope }}</p>
 
-    {{ form_start(form) }}
-    {{ form_errors(form) }}
+    <div class="container" style="margin-top: 150px;">
+        <div class="row">
+            <div class="col-12 col-lg-6 offset-lg-3">
+                <div class="text-center p-5 bg-white">
+                    <form name="form1" id="form1" method="post" action="">
+                        {{ form_widget(form._token) }}
+                        <p class="text-left">{{ 'api.admin.oauth.allow__confirm_message'|trans({'%shop_name%': "<a href=\"#{path('homepage')}\">#{BaseInfo.shop_name}</a>"})|raw }}</p>
+                        <p class="text-left">{{ 'api.admin.oauth.allow__confirm_description'|trans }}</p>
+                        <ul>
+                        {% for scope in scopes %}
+                        <li class="text-left">{{ "api.admin.oauth.scope.#{scope}.description"|trans({'%shop_name%': "<a href=\"#{path('homepage')}\">#{BaseInfo.shop_name}</a>"})|raw }}</li>
+                        {% endfor %}
+                        </ul>
 
-    {{ form_row(form.client_id) }}
-    {{ form_row(form.client_secret) }}
-    {{ form_row(form.redirect_uri) }}
-    {{ form_row(form.response_type) }}
-    {{ form_row(form.state) }}
-    {{ form_row(form.scope) }}
-    <div class="col-auto">
-        <button class="btn btn-ec-conversion px-5" type="submit">許可</button>
-    </div>
-    {{ form_end(form) }}
+                        {{ form_row(form.client_id) }}
+                        {{ form_row(form.client_secret) }}
+                        {{ form_row(form.redirect_uri) }}
+                        {{ form_row(form.response_type) }}
+                        {{ form_row(form.state) }}
+                        {{ form_row(form.scope) }}
 
+                        <div class="col-auto">
+                            {{ form_widget(form.approve, {'label':'api.admin.oauth.allow'}) }}
+                            {{ form_widget(form.deny, {'label':'api.admin.oauth.deny', 'attr':{'class':'btn btn-light'}}) }}
+                        </div>
+                    </form>
+                </div>
+            </div>
+            <div class="col-12">
+                <p class="text-center mt-3">
+                    <small>Copyright &copy; 2000-{{ "now"|date("Y") }} EC-CUBE CO.,LTD. All Rights Reserved.</small>
+                </p>
+            </div>
+        </div>
+    </div>
 {% endblock %}
diff --git a/Tests/Web/Admin/OAuth2Bundle/AuthorizationControllerTest.php b/Tests/Web/Admin/OAuth2Bundle/AuthorizationControllerTest.php
@@ -15,6 +15,7 @@
 
 use Eccube\Common\Constant;
 use Eccube\Tests\Web\Admin\AbstractAdminWebTestCase;
+use Symfony\Component\HttpFoundation\Response;
 use Trikoder\Bundle\OAuth2Bundle\Model\Client;
 
 class AuthorizationControllerTest extends AbstractAdminWebTestCase
@@ -74,16 +75,72 @@ public function testRoutingAdminOauth2Authorize_権限移譲を許可()
                 'response_type' => 'code',
                 'scope' => 'read',
                 'state' => 'xxx',
+                'approve' => '',
                 Constant::TOKEN_NAME => 'dummy',
             ],
         ];
 
-        $crawler = $this->client->request(
+        $this->client->request(
+            'POST', $authorize_url,
+            $parameters
+        );
+
+        /** @var Response $response */
+        $response = $this->client->getResponse();
+        $this->assertTrue($response->isRedirection());
+
+        $callbackParams = $this->parseCallbackParams($response);
+
+        self::assertFalse(isset($callbackParams['error']));
+        self::assertTrue(isset($callbackParams['code']));
+
+    }
+
+    public function testRoutingAdminOauth2Authorize_権限移譲を許可しない()
+    {
+        /** @var Client $Client */
+        $Client = $this->entityManager->getRepository(Client::class)->findOneBy([]);
+        $authorize_url = $this->generateUrl(
+            'oauth2_authorize',
+            [
+                'client_id' => $Client->getIdentifier(),
+                'redirect_uri' => (string) current($Client->getRedirectUris()),
+                'response_type' => 'code',
+                'scope' => 'read',
+                'state' => 'xxx',
+            ]
+        );
+
+        $this->client->request('GET', $authorize_url);
+
+        $parameters = [
+            'oauth_authorization' => [
+                'client_id' => $Client->getIdentifier(),
+                'client_secret' => $Client->getSecret(),
+                'redirect_uri' => current($Client->getRedirectUris()),
+                'response_type' => 'code',
+                'scope' => 'read',
+                'state' => 'xxx',
+                'deny' => '',
+                Constant::TOKEN_NAME => 'dummy',
+            ],
+        ];
+
+        $this->client->request(
             'POST', $authorize_url,
             $parameters
         );
 
-        $this->assertTrue($this->client->getResponse()->isRedirection());
+        /** @var Response $response */
+        $response = $this->client->getResponse();
+        $this->assertTrue($response->isRedirection());
+
+        $redirectUrl = $response->headers->get('Location');
+        self::assertStringStartsWith((string) $Client->getRedirectUris()[0], $redirectUrl);
+
+        $callbackParams = $this->parseCallbackParams($response);
+        self::assertEquals('access_denied', $callbackParams['error']);
+
     }
 
     public function testRoutingAdminOauth2Authorize_権限移譲を許可_パラメータが足りない場合()
@@ -107,4 +164,13 @@ public function testRoutingAdminOauth2Authorize_権限移譲を許可_パラメ
 
         $this->assertFalse($this->client->getResponse()->isRedirection());
     }
+
+    private function parseCallbackParams(Response $response)
+    {
+        $url = parse_url($response->headers->get('Location'));
+        $redirectParams = [];
+        parse_str($url['query'], $redirectParams);
+
+        return $redirectParams;
+    }
 }