You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/08/17 09:13:42 UTC

[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #330: feat(authentication): create authentication module

juzhiyuan commented on a change in pull request #330:
URL: https://github.com/apache/apisix-dashboard/pull/330#discussion_r471343994



##########
File path: src/pages/User/Login.tsx
##########
@@ -0,0 +1,107 @@
+import React, { useState } from 'react';

Review comment:
       Please add ASF header.

##########
File path: src/pages/User/Login.tsx
##########
@@ -0,0 +1,107 @@
+import React, { useState } from 'react';
+import { Button, notification, Tabs } from 'antd';
+import { DefaultFooter } from '@ant-design/pro-layout';
+import { SelectLang } from '@@/plugin-locale/SelectLang';
+import { Link, useIntl, history } from 'umi';
+import LoginMethodPassword from '@/pages/User/components/LoginMethodPassword';
+import LoginMethodExample from '@/pages/User/components/LoginMethodExample';
+import { UserModule } from '@/pages/User/typing';
+import logo from '@/assets/logo.svg';
+import { SettingOutlined } from '@ant-design/icons';
+import styles from './Login.less';
+import { getUrlQuery } from '@/helpers';
+
+const Tab = Tabs.TabPane;
+
+/**
+ * Login Methods List
+ */
+const loginMethods: UserModule.LoginMethod[] = [LoginMethodPassword, LoginMethodExample];
+
+/**
+ * User Login Page
+ * @constructor
+ */
+const Page: React.FC = () => {
+  const { formatMessage } = useIntl();
+  const [loginMethod, setLoginMethod] = useState(loginMethods[0]);
+
+  const onSettingsClick = () => {
+    history.replace(`/settings?redirect=${encodeURIComponent(history.location.pathname)}`);
+  };
+
+  const onTabChange = (activeKey: string) => {
+    loginMethods.forEach((item, index) => {
+      if (activeKey === item.id) setLoginMethod(loginMethods[index]);
+    });
+  };
+
+  const onSubmit = () => {
+    loginMethod.checkData().then((validate) => {
+      if (validate) {
+        loginMethod.submit(loginMethod.getData()).then((response) => {
+          if (response.status) {
+            notification.success({
+              message: formatMessage({ id: 'component.status.success' }),
+              description: response.message,
+              duration: 1,
+              onClose: () => {
+                const redirect = getUrlQuery('redirect');
+                history.replace(redirect ? decodeURIComponent(redirect) : '/');
+              },
+            });
+          } else {
+            notification.error({
+              message: formatMessage({ id: 'component.status.fail' }),
+              description: response.message,
+            });
+          }
+        });
+      }
+    });
+  };
+
+  if (localStorage.getItem('token')) {
+    history.replace('/');
+    return <></>;
+  }
+  return (
+    <div className={styles.container}>
+      <div className={styles.lang}>
+        <div className={styles.settings} onClick={onSettingsClick}>
+          <SettingOutlined />
+        </div>
+        <SelectLang />
+      </div>
+      <div className={styles.content}>
+        <div className={styles.top}>
+          <div className={styles.header}>
+            <Link to="/">
+              <img alt="logo" className={styles.logo} src={logo} />
+            </Link>
+          </div>
+          <div className={styles.desc}>
+            APISIX Dashboard

Review comment:
       Apache APISIX Dashboard

##########
File path: api/filter/authentication.go
##########
@@ -0,0 +1,50 @@
+package filter
+
+import (
+	"github.com/apisix/manager-api/conf"

Review comment:
       1. If we need those un-existing URL?
   2. Please have a License check for Line 6 ~ 9, It’s ok to use Apache 2.0, MIT, etc.

##########
File path: src/app.tsx
##########
@@ -32,6 +32,11 @@ export async function getInitialState(): Promise<{
   currentUser?: API.CurrentUser;
   settings?: LayoutSettings;
 }> {
+  const token = localStorage.getItem('token') || '';
+  if (token === '') {

Review comment:
       String or Undefined I think, don’t need the extra default empty string.

##########
File path: src/pages/User/Logout.tsx
##########
@@ -0,0 +1,28 @@
+import React from 'react';

Review comment:
       ASF Header.

##########
File path: api/errno/error.go
##########
@@ -84,6 +85,9 @@ var (
 	ApisixConsumerUpdateError = Message{"010703", "APISIX Consumer update failed", 500}
 	ApisixConsumerDeleteError = Message{"010704", "APISIX Consumer delete failed", 500}
 	DuplicateUserName         = Message{"010705", "Duplicate consumer username", 400}
+
+	// 99 authentication

Review comment:
       @nic-chen @gxthrj OK, this PR uses 99 as Authentication code.

##########
File path: api/route/authentication.go
##########
@@ -0,0 +1,54 @@
+package route
+
+import (
+	"github.com/apisix/manager-api/conf"
+	"github.com/apisix/manager-api/errno"
+	jwt "github.com/dgrijalva/jwt-go"
+	"github.com/gin-gonic/gin"
+	"net/http"
+	"time"
+)
+
+type UserSession struct {
+	Token string `json:"token"`
+}
+
+func AppendAuthentication(r *gin.Engine) *gin.Engine {
+	r.POST("/user/login", userLogin)
+	return r
+}
+
+func userLogin(c *gin.Context) {
+	username := c.PostForm("username")
+	password := c.PostForm("password")
+
+	if username == "" {
+		c.AbortWithStatusJSON(http.StatusBadRequest, errno.FromMessage(errno.InvalidParamDetail, "username is needed").Response())
+		return
+	}
+	if password == "" {
+		c.AbortWithStatusJSON(http.StatusBadRequest, errno.FromMessage(errno.InvalidParamDetail, "password is needed").Response())
+		return
+	}
+
+	user := conf.UserList[username]
+	if username != user.Username || password != user.Password {
+		c.AbortWithStatusJSON(http.StatusUnauthorized, errno.FromMessage(errno.AuthenticationUserError).Response())
+		return

Review comment:
       Don’t need this return I think? Because it’s in a condition block and has no other logics after that.

##########
File path: api/go.mod
##########
@@ -4,12 +4,15 @@ go 1.13
 
 require (
 	github.com/api7/apitest v1.4.9
+	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/gin-contrib/pprof v1.3.0
+	github.com/gin-contrib/sessions v0.0.3
 	github.com/gin-gonic/gin v1.6.3
-	github.com/go-sql-driver/mysql v1.5.0
+	github.com/go-sql-driver/mysql v1.5.0 // indirect
 	github.com/jinzhu/gorm v1.9.12
 	github.com/satori/go.uuid v1.2.0
 	github.com/sirupsen/logrus v1.6.0
+	github.com/steinfletcher/apitest v1.4.9 // indirect

Review comment:
       Please have a License Check if needed.

##########
File path: src/pages/User/Login.tsx
##########
@@ -0,0 +1,107 @@
+import React, { useState } from 'react';
+import { Button, notification, Tabs } from 'antd';
+import { DefaultFooter } from '@ant-design/pro-layout';
+import { SelectLang } from '@@/plugin-locale/SelectLang';
+import { Link, useIntl, history } from 'umi';
+import LoginMethodPassword from '@/pages/User/components/LoginMethodPassword';
+import LoginMethodExample from '@/pages/User/components/LoginMethodExample';
+import { UserModule } from '@/pages/User/typing';
+import logo from '@/assets/logo.svg';
+import { SettingOutlined } from '@ant-design/icons';
+import styles from './Login.less';
+import { getUrlQuery } from '@/helpers';
+
+const Tab = Tabs.TabPane;
+
+/**
+ * Login Methods List
+ */
+const loginMethods: UserModule.LoginMethod[] = [LoginMethodPassword, LoginMethodExample];
+
+/**
+ * User Login Page
+ * @constructor
+ */
+const Page: React.FC = () => {
+  const { formatMessage } = useIntl();
+  const [loginMethod, setLoginMethod] = useState(loginMethods[0]);
+
+  const onSettingsClick = () => {
+    history.replace(`/settings?redirect=${encodeURIComponent(history.location.pathname)}`);
+  };
+
+  const onTabChange = (activeKey: string) => {
+    loginMethods.forEach((item, index) => {
+      if (activeKey === item.id) setLoginMethod(loginMethods[index]);
+    });
+  };
+
+  const onSubmit = () => {
+    loginMethod.checkData().then((validate) => {
+      if (validate) {
+        loginMethod.submit(loginMethod.getData()).then((response) => {
+          if (response.status) {
+            notification.success({
+              message: formatMessage({ id: 'component.status.success' }),
+              description: response.message,
+              duration: 1,
+              onClose: () => {
+                const redirect = getUrlQuery('redirect');
+                history.replace(redirect ? decodeURIComponent(redirect) : '/');
+              },
+            });
+          } else {
+            notification.error({
+              message: formatMessage({ id: 'component.status.fail' }),
+              description: response.message,
+            });
+          }
+        });
+      }
+    });
+  };
+
+  if (localStorage.getItem('token')) {
+    history.replace('/');
+    return <></>;

Review comment:
       Return null is fine.

##########
File path: src/helpers.tsx
##########
@@ -1,22 +1,7 @@
-/*

Review comment:
       Please keep this Header.

##########
File path: src/pages/User/Login.less
##########
@@ -0,0 +1,118 @@
+@import '~antd/es/style/themes/default.less';

Review comment:
       Please add ASF header.

##########
File path: src/pages/User/Login.less
##########
@@ -0,0 +1,118 @@
+@import '~antd/es/style/themes/default.less';
+
+.container {
+  display: flex;
+  flex-direction: column;
+  height: 100vh;
+  overflow: auto;
+  background: @layout-body-background;
+}
+
+.lang {
+  width: 100%;
+  height: 42px;
+  line-height: 42px;
+  text-align: right;
+}
+
+.settings {
+  display: inline-flex;
+  align-items: center;
+  justify-content: center;
+  padding: 12px;
+  font-size: 16px;
+  vertical-align: middle;
+  cursor: pointer;
+}
+
+.settings-icon {
+  font-size: 18px;
+}
+
+.content {
+  flex: 1;
+  padding: 32px 0;
+}
+
+@media (min-width: @screen-md-min) {
+  .container {
+    background-image: url('https://gw.alipayobjects.com/zos/rmsportal/TVYTbAXWheQpRcWDaDMu.svg');

Review comment:
       This link is invalid, please remove it or use the local file.

##########
File path: src/pages/User/Logout.tsx
##########
@@ -0,0 +1,28 @@
+import React from 'react';
+import LoginMethodPassword from '@/pages/User/components/LoginMethodPassword';
+import LoginMethodExample from '@/pages/User/components/LoginMethodExample';
+import { UserModule } from '@/pages/User/typing';
+import { getUrlQuery } from '@/helpers';
+
+/**
+ * Login Methods List
+ */
+const loginMethods: UserModule.LoginMethod[] = [LoginMethodPassword, LoginMethodExample];
+
+/**
+ * User Logout Page
+ * @constructor
+ */
+const Page: React.FC = () => {
+  // run all logout method
+  loginMethods.forEach((item) => {
+    item.logout();
+  });
+
+  const redirect = getUrlQuery('redirect');
+  window.location.href = `/user/login${redirect ? `?redirect=${redirect}` : ''}`;
+
+  return <div />;

Review comment:
       Maybe return null or Fragment would be better?

##########
File path: src/pages/User/components/LoginMethodPassword.tsx
##########
@@ -0,0 +1,119 @@
+import React from 'react';

Review comment:
       ASF Header.

##########
File path: src/pages/User/Logout.tsx
##########
@@ -0,0 +1,28 @@
+import React from 'react';
+import LoginMethodPassword from '@/pages/User/components/LoginMethodPassword';
+import LoginMethodExample from '@/pages/User/components/LoginMethodExample';
+import { UserModule } from '@/pages/User/typing';
+import { getUrlQuery } from '@/helpers';
+
+/**
+ * Login Methods List
+ */
+const loginMethods: UserModule.LoginMethod[] = [LoginMethodPassword, LoginMethodExample];
+
+/**
+ * User Logout Page
+ * @constructor
+ */
+const Page: React.FC = () => {
+  // run all logout method
+  loginMethods.forEach((item) => {
+    item.logout();
+  });
+
+  const redirect = getUrlQuery('redirect');
+  window.location.href = `/user/login${redirect ? `?redirect=${redirect}` : ''}`;
+
+  return <div />;

Review comment:
       Maybe return null or Fragment would be better?

##########
File path: src/pages/User/components/LoginMethodExample.tsx
##########
@@ -0,0 +1,27 @@
+import React from 'react';

Review comment:
       ASF Header.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org