From 6a5da2a2e7604d65f26855629a0abcbb02e98a02 Mon Sep 17 00:00:00 2001
From: Alexis POYEN <apoyen@grandlyon.com>
Date: Fri, 23 Apr 2021 11:07:37 +0200
Subject: [PATCH] Fix: some code optimization
---
internal/auth/inmemory.go | 26 ++++++++++++++------------
internal/auth/oauth2.go | 2 +-
internal/rootmux/rootmux.go | 3 +--
internal/rootmux/rootmux_test.go | 14 ++++++--------
internal/rootmux/unlogged_test.go | 8 ++++----
5 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/internal/auth/inmemory.go b/internal/auth/inmemory.go
index e3c7cfb..879300b 100644
--- a/internal/auth/inmemory.go
+++ b/internal/auth/inmemory.go
@@ -96,17 +96,20 @@ func (d *DataHandler) AddUser(w http.ResponseWriter, req *http.Request) {
http.Error(w, err.Error(), 400)
return
}
+
+ // Check login don't already exist
+ for _, val := range users {
+ if newUser.Login == val.Login {
+ http.Error(w, "login already exists", 400)
+ return
+ }
+ }
+
// Encrypt the password with bcrypt
if newUser.Password == "" {
http.Error(w, "passwords cannot be blank", 400)
return
- }
- if newUser.Role == "ADMIN" {
- newUser.IsAdmin = true
} else {
- newUser.IsAdmin = false
- }
- if newUser.Password != "" {
hash, err := bcrypt.GenerateFromPassword([]byte(newUser.Password), bcrypt.DefaultCost)
if err != nil {
http.Error(w, err.Error(), 400)
@@ -115,13 +118,12 @@ func (d *DataHandler) AddUser(w http.ResponseWriter, req *http.Request) {
newUser.PasswordHash = string(hash)
newUser.Password = ""
}
- // Check login don't already exist
- for _, val := range users {
- if newUser.Login == val.Login {
- http.Error(w, "login already exists", 400)
- return
- }
+ if newUser.Role == "ADMIN" {
+ newUser.IsAdmin = true
+ } else {
+ newUser.IsAdmin = false
}
+
d.createUser(newUser)
d.db.Last(&newUser)
json.NewEncoder(w).Encode(newUser)
diff --git a/internal/auth/oauth2.go b/internal/auth/oauth2.go
index e65e357..8e1189d 100644
--- a/internal/auth/oauth2.go
+++ b/internal/auth/oauth2.go
@@ -59,7 +59,7 @@ func (m Manager) HandleOAuth2Login(w http.ResponseWriter, r *http.Request) {
// Generate state and store it in cookie
oauthStateString, err := common.GenerateRandomString(16)
if err != nil {
- log.Logger.Fatalf("Error generating OAuth2 strate string :%v\n", err)
+ log.Logger.Fatalf("Error generating OAuth2 state string :%v\n", err)
}
tokens.Manager.StoreData(oauthStateString, m.Hostname, oAuth2StateKey, 30*time.Second, w)
url := m.Config.AuthCodeURL(oauthStateString)
diff --git a/internal/rootmux/rootmux.go b/internal/rootmux/rootmux.go
index c0dfb44..2cc76e7 100644
--- a/internal/rootmux/rootmux.go
+++ b/internal/rootmux/rootmux.go
@@ -6,9 +6,8 @@ import (
"forge.grandlyon.com/systemes-dinformation/project-template/sdk-go/internal/auth"
"forge.grandlyon.com/systemes-dinformation/project-template/sdk-go/internal/models"
- "forge.grandlyon.com/systemes-dinformation/project-template/sdk-go/pkg/middlewares"
-
"forge.grandlyon.com/systemes-dinformation/project-template/sdk-go/pkg/common"
+ "forge.grandlyon.com/systemes-dinformation/project-template/sdk-go/pkg/middlewares"
)
// RootMux represents the main controller of the application
diff --git a/internal/rootmux/rootmux_test.go b/internal/rootmux/rootmux_test.go
index 6a744af..6a2c1bb 100644
--- a/internal/rootmux/rootmux_test.go
+++ b/internal/rootmux/rootmux_test.go
@@ -89,28 +89,26 @@ func appTests(t *testing.T) {
json.Unmarshal([]byte(response), &token)
xsrfHeader := map[string]string{"XSRF-TOKEN": token.XSRFToken}
- const apiOperation1 = "/api/Operations/1"
- const apiBankAccount1 = "/api/BankAccounts/1"
// Add invalid operation between client and Bakery must be refused with 417 (Expectation failed)
do("POST", "/api/Operations", xsrfHeader, `{"Debtor":1,"Amount":-1789,"Creditor":2}`, 417, "Not enough money")
// Add an operation between Dupond and Bakery and verify that bank accounts are updated and opposite operation is created
do("POST", "/api/Operations", xsrfHeader, `{"Debtor":1,"Amount":-100,"Creditor":2}`, 200, "")
- do("GET", apiOperation1, xsrfHeader, "", 200, `{"ID":1,"Debtor":1,"Amount":-100`)
+ do("GET", "/api/Operations/1", xsrfHeader, "", 200, `{"ID":1,"Debtor":1,"Amount":-100`)
do("GET", "/api/Operations/2", xsrfHeader, "", 200, `{"ID":2,"Debtor":2,"Amount":100`)
- do("GET", apiBankAccount1, xsrfHeader, "", 200, `{"ID":1,"Number":"01-01","UserClientID":1,"Type":"checking-account","Amount":358,"BankOverdraft":-100,"Operations":[{"ID":1,"Debtor":1,"Amount":-100,"Date":"`)
+ do("GET", "/api/BankAccounts/1", xsrfHeader, "", 200, `{"ID":1,"Number":"01-01","UserClientID":1,"Type":"checking-account","Amount":358,"BankOverdraft":-100,"Operations":[{"ID":1,"Debtor":1,"Amount":-100,"Date":"`)
do("GET", "/api/BankAccounts/2", xsrfHeader, "", 200, `{"ID":2,"Number":"02-01","UserClientID":2,"Type":"checking-account","Amount":4845,"BankOverdraft":-500,"Operations":[{"ID":2,"Debtor":2,"Amount":100,"Date":`)
// Try to delete the first operation, the opposite operation should also have been deleted and bank accounts updated
- do("DELETE", apiOperation1, xsrfHeader, ``, 200, "")
- do("GET", apiOperation1, xsrfHeader, "", 404, `id does not exist`)
+ do("DELETE", "/api/Operations/1", xsrfHeader, ``, 200, "")
+ do("GET", "/api/Operations/1", xsrfHeader, "", 404, `id does not exist`)
do("GET", "/api/Operations/2", xsrfHeader, "", 404, `id does not exist`)
- do("GET", apiBankAccount1, xsrfHeader, "", 200, `{"ID":1,"Number":"01-01","UserClientID":1,"Type":"checking-account","Amount":458,"BankOverdraft":-100,"Operations":[]}`)
+ do("GET", "/api/BankAccounts/1", xsrfHeader, "", 200, `{"ID":1,"Number":"01-01","UserClientID":1,"Type":"checking-account","Amount":458,"BankOverdraft":-100,"Operations":[]}`)
do("GET", "/api/BankAccounts/2", xsrfHeader, "", 200, `{"ID":2,"Number":"02-01","UserClientID":2,"Type":"checking-account","Amount":4745,"BankOverdraft":-500,"Operations":[]}`)
// Delete a client should also delete his banks accounts
do("DELETE", "/api/UserClients/1", xsrfHeader, ``, 200, "")
- do("GET", apiBankAccount1, xsrfHeader, "", 404, `id does not exist`)
+ do("GET", "/api/BankAccounts/1", xsrfHeader, "", 404, `id does not exist`)
}
// Do an OAuth2 login with an known admin
diff --git a/internal/rootmux/unlogged_test.go b/internal/rootmux/unlogged_test.go
index dedec8e..3344705 100644
--- a/internal/rootmux/unlogged_test.go
+++ b/internal/rootmux/unlogged_test.go
@@ -52,11 +52,11 @@ func UnLoggedUserTests(t *testing.T) {
do("POST", "/api/Operations", noH, `{"Debtor":1,"Amount":-100,"Creditor":3}`, 401, errorExtractingToken)
// Unlogged user should not be able to delete an Operation
- do("DELETE", "/api/Operations/1", noH, ``, 401, "error extracting token")
+ do("DELETE", "/api/Operations/1", noH, ``, 401, errorExtractingToken)
// Unlogged user should not be able to delete a BankAccount
- do("DELETE", "/api/BankAccounts/2", noH, ``, 401, "error extracting token")
+ do("DELETE", "/api/BankAccounts/2", noH, ``, 401, errorExtractingToken)
// Unlogged user should not be able to delete a Client
- do("DELETE", "/api/UserClients/2", noH, ``, 401, "error extracting token")
+ do("DELETE", "/api/UserClients/2", noH, ``, 401, errorExtractingToken)
// Unlogged user should not be able to delete a Banker
- do("DELETE", "/api/UserBankers/2", noH, ``, 401, "error extracting token")
+ do("DELETE", "/api/UserBankers/2", noH, ``, 401, errorExtractingToken)
}
--
GitLab