Merge branch 'master' of https://github.com/CS2103-AY1819S1-F10-2/main …

…into update-loanfield
CS2103-AY1819S1-F10-2 · Oct 25, 2018 · 123bbbc · 123bbbc
2 parents d5de5af + c663e60
commit 123bbbc
Show file tree

Hide file tree

Showing 36 changed files with 105 additions and 117 deletions.
diff --git a/docs/DeveloperGuide.adoc b/docs/DeveloperGuide.adoc
@@ -318,6 +318,61 @@ image::UndoRedoActivityDiagram.png[width="650"]
 ** Cons: Requires dealing with commands that have already been undone: We must remember to skip these commands. Violates Single Responsibility Principle and Separation of Concerns as `HistoryManager` now needs to do two different things.
 // end::undoredo[]
 
+// tag::adminauthentication[]
+=== Admin Authentication
+Before critical actions such as deleting a loan can be performed, admin authentication
+should be required. This ensures that only authorized users are able to perform critical actions.
+This is done by requiring a password before performing a critical action.
+
+* Current Implementation *
+There are three stages in this implementation:
+
+===== Allowing the `delete` command to accept a password
+1. To allow `delete` command to accept a password, we set a stub password of `a12345` and require all `delete` command to take in a password argument.
+* When password is wrong, we do not perform the deletion of the loans.
+* Only when password matches with stub password will the loan be deleted.
+2. Existing tests were edited to include and check for a valid password before deletion.
+
+[NOTE]
+The default password for a new app is `a12345`.
+
+The following sequence diagram shows how the new `delete` operation works.
+
+image::deleteLoanWithPass.png[width="1000"]
+
+===== Allow changing of master password
+
+Given below is an example usage scenario and how the `setpass` mechanism behaves at each step.
+
+Step 1. The user launches the application for the first time. `UserPref` file will initialise the password as `a12345`.
+
+Step 2. The user executes `setpass a12345 newP4sS` command to change the password to `newP4sS`.
+
+Step 3. `Password` class will encrypt the password, and call Model#setPass, that changes the password of the application in `UserPref`.
+
+[NOTE]
+If the current password input is wrong, it will not call Model#setPass, so the UserPref state will not be saved.
+
+[NOTE]
+If the current password input the same as the new password input, it will not call Model#setPass, so the UserPref state will not be saved.
+
+Step 4. Password in `UserPref` is set to new password input.
+
+The following sequence diagram shows how the `setpass` operation works:
+
+image::setPasswordLogic.png[width="1000"]
+
+image::setPasswordModel.png[width="1000"]
+
+===== Encrypt current password for security
+To ensure that others are unable to retrieve the app's password from the preference.json, we encrypt the password before storing. Decryption of password can only be done by the `Password` class.
+
+This is currently done by appending `-encrypt` to the password, and removing it when
+decrypting. For version 2.0, we plan to utilise existing libraries to encrypt and
+decrypt our stored password.
+
+// end::userconfirmation[]
+
 === Return a Loan feature
 ==== Proposed Implementation
 
@@ -524,10 +579,16 @@ When showing a new loan, it will do the following operations:
 
 * `Nric#getCensored()` -- censor the Nric value and hide the first five digital numbers. Then it returns the censored String.
 
+* `Nric#doCensoring(int length)` -- Take in the length of the Nric String and return the censored part ("x" part) according to the length.
+
 * `Phone#getCensored()` -- censor the phone number and hide the first five digital numbers. Then it returns the censored String.
 
+* `Phone#doCensoring(int length)` -- Take in the length of the Phone String and return the censored part ("x" part) according to the length.
+
 * `Email#getCensored()` -- censor the email address and hide every characters except the last two characters in the local-part and the domain.
 
+* `Email#doCensoring(int length)` -- Take in the length of the Email String and return the censored part ("x" part) according to the length.
+
 Given below is an example usage scenario and how this mechanism behaves at each step.
 
 **Step1.** The user adds a new loan to the LoanBook. A new `LoanCard` object will be created. +
@@ -537,6 +598,7 @@ The `LoanCard` object will contain information on the loan: `LoanID`, `BikeID`,
 For example: `name.setText(loan.getName().value);` will directly assign the name String of this loan to the `name` label. +
 
 However, the values of `Nric`, `Phone` and `Email` need censoring before assigning their values, so they will call their own `getCensored()` method in their class. +
+`getCensored()` in each of these class will call their corresponding `doCensoring(int)` method. Then combine the censored part and remain part and return. +
 For example: `phone.setText(loan.getPhone().getCensored().value);` will censor the value of the phone String of this loan and then assign the censored value to the `phone` label.
 
 The following sequence diagram shows how this operation works:
@@ -545,7 +607,7 @@ image::HideInfoSequenceDiagrams.png[]
 
 ==== Design Considerations
 
-**Aspect: How to execute getCensored()**
+**Aspect: How to execute getCensored() and doCensoring(int)**
 
 * **Alternative 1 (current choice)**: each class implements from `Censor` interface.
 ** Pros: Easy to implement.

diff --git a/docs/diagrams/HideInfoSequenceDiagrams.pptx b/docs/diagrams/HideInfoSequenceDiagrams.pptx
diff --git a/docs/diagrams/HighLevelSequenceDiagrams.pptx b/docs/diagrams/HighLevelSequenceDiagrams.pptx
diff --git a/docs/diagrams/LoanListPanelView.pptx b/docs/diagrams/LoanListPanelView.pptx
diff --git a/docs/diagrams/LogicComponentClassDiagram.pptx b/docs/diagrams/LogicComponentClassDiagram.pptx
diff --git a/docs/diagrams/LogicComponentSequenceDiagram.pptx b/docs/diagrams/LogicComponentSequenceDiagram.pptx
diff --git a/docs/diagrams/StorageComponentClassDiagram.pptx b/docs/diagrams/StorageComponentClassDiagram.pptx
diff --git a/docs/diagrams/UiComponentClassDiagram.pptx b/docs/diagrams/UiComponentClassDiagram.pptx
diff --git a/docs/diagrams/UndoRedoActivityDiagram.pptx b/docs/diagrams/UndoRedoActivityDiagram.pptx
diff --git a/docs/diagrams/UndoRedoExecuteUndoStateListDiagram.pptx b/docs/diagrams/UndoRedoExecuteUndoStateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand1StateListDiagram.pptx b/docs/diagrams/UndoRedoNewCommand1StateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand2StateListDiagram.pptx b/docs/diagrams/UndoRedoNewCommand2StateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand3StateListDiagram.pptx b/docs/diagrams/UndoRedoNewCommand3StateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoNewCommand4StateListDiagram.pptx b/docs/diagrams/UndoRedoNewCommand4StateListDiagram.pptx
diff --git a/docs/diagrams/UndoRedoSequenceDiagram.pptx b/docs/diagrams/UndoRedoSequenceDiagram.pptx
diff --git a/docs/diagrams/UndoRedoStartingStateListDiagram.pptx b/docs/diagrams/UndoRedoStartingStateListDiagram.pptx
diff --git a/docs/images/DeleteLoanSdForLogic.png b/docs/images/DeleteLoanSdForLogic.png
diff --git a/docs/images/HideInfoSequenceDiagrams.png b/docs/images/HideInfoSequenceDiagrams.png
diff --git a/docs/images/LoanListPanelView.png b/docs/images/LoanListPanelView.png
diff --git a/docs/images/LogicClassDiagram.png b/docs/images/LogicClassDiagram.png
diff --git a/docs/images/SDforDeleteLoan.png b/docs/images/SDforDeleteLoan.png
diff --git a/docs/images/SDforDeleteLoanEventHandling.png b/docs/images/SDforDeleteLoanEventHandling.png
diff --git a/docs/images/StorageClassDiagram.png b/docs/images/StorageClassDiagram.png
diff --git a/docs/images/UiClassDiagram.png b/docs/images/UiClassDiagram.png
diff --git a/docs/images/UndoRedoActivityDiagram.png b/docs/images/UndoRedoActivityDiagram.png
diff --git a/docs/images/UndoRedoExecuteUndoStateListDiagram.png b/docs/images/UndoRedoExecuteUndoStateListDiagram.png
diff --git a/docs/images/UndoRedoNewCommand1StateListDiagram.png b/docs/images/UndoRedoNewCommand1StateListDiagram.png
diff --git a/docs/images/UndoRedoNewCommand2StateListDiagram.png b/docs/images/UndoRedoNewCommand2StateListDiagram.png
diff --git a/docs/images/UndoRedoNewCommand3StateListDiagram.png b/docs/images/UndoRedoNewCommand3StateListDiagram.png
diff --git a/docs/images/UndoRedoNewCommand4StateListDiagram.png b/docs/images/UndoRedoNewCommand4StateListDiagram.png
diff --git a/docs/images/UndoRedoSequenceDiagram.png b/docs/images/UndoRedoSequenceDiagram.png
diff --git a/docs/images/UndoRedoStartingStateListDiagram.png b/docs/images/UndoRedoStartingStateListDiagram.png
diff --git a/docs/images/deleteLoanWithPass.png b/docs/images/deleteLoanWithPass.png
diff --git a/docs/images/setPasswordLogic.png b/docs/images/setPasswordLogic.png
diff --git a/docs/images/setPasswordModel.png b/docs/images/setPasswordModel.png
diff --git a/src/main/java/seedu/address/storage/XmlAdaptedLoan.java b/src/main/java/seedu/address/storage/XmlAdaptedLoan.java
@@ -5,6 +5,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import javax.xml.bind.annotation.XmlElement;
@@ -86,6 +87,23 @@ public XmlAdaptedLoan(String name,
         if (tagged != null) {
             this.tagged = new ArrayList<>(tagged);
         }
+        this.loanStatus = loanStatus;
+    }
+
+    /**
+     * Constructs an {@code XmlAdaptedLoan} with the given loan details.
+     * This constructor is called if no loanStatus is given
+     */
+    public XmlAdaptedLoan(String name,
+                          String nric,
+                          String phone,
+                          String email,
+                          String address,
+                          String bike,
+                          String rate,
+                          String time,
+                          List<XmlAdaptedTag> tagged) {
+        this(name, nric, phone, email, address, bike, rate, time, "ONGOING", tagged);
     }
 
     /**
@@ -127,119 +145,27 @@ public XmlAdaptedLoan(Loan source) {
     }
 
     /**
-     * Throws an {@code IllegalValueException} if {@code name} does not exist or is not valid.
-     *
-     * @throws IllegalValueException
-     */
-    private void checkNameValid() throws IllegalValueException {
-        if (name == null) {
-            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Name.class.getSimpleName()));
-        }
-        if (!Name.isValidName(name)) {
-            throw new IllegalValueException(Name.MESSAGE_NAME_CONSTRAINTS);
-        }
-    }
-
-    /**
-     * Throws an {@code IllegalValueException} if {@code nric} does not exist or is not valid.
+     * Throws an {@code IllegalValueException} if {@code field} does not exist or is not valid.
      *
+     * @param field The data field of the Loan class to check for.
+     * @param fieldClass The class which the data field should belong to.
+     * @param isValid A predicate to check if {@code field} is valid.
+     * @param msgConstraints A message to display to the user if {@code field} is not valid.
      * @throws IllegalValueException
      */
-    private void checkNricValid() throws IllegalValueException {
-        if (nric == null) {
-            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Nric.class.getSimpleName()));
+    private void checkFieldValid(
+            String field,
+            Class fieldClass,
+            Predicate<String> isValid,
+            String msgConstraints) throws IllegalValueException {
+
+        if (field == null) {
+            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, fieldClass.getSimpleName()));
         }
-        if (!Nric.isValidNric(nric)) {
-            throw new IllegalValueException(Nric.MESSAGE_NRIC_CONSTRAINTS);
+        if (!isValid.test(field)) {
+            throw new IllegalValueException(msgConstraints);
         }
     }
-
-    /**
-     * Throws an {@code IllegalValueException} if {@code phone} does not exist or is not valid.
-     *
-     * @throws IllegalValueException
-     */
-    private void checkPhoneValid() throws IllegalValueException {
-        if (phone == null) {
-            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Phone.class.getSimpleName()));
-        }
-        if (!Phone.isValidPhone(phone)) {
-            throw new IllegalValueException(Phone.MESSAGE_PHONE_CONSTRAINTS);
-        }
-    }
-
-    /**
-     * Throws an {@code IllegalValueException} if {@code email} does not exist or is not valid.
-     *
-     * @throws IllegalValueException
-     */
-    private void checkEmailValid() throws IllegalValueException {
-        if (email == null) {
-            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Email.class.getSimpleName()));
-        }
-        if (!Email.isValidEmail(email)) {
-            throw new IllegalValueException(Email.MESSAGE_EMAIL_CONSTRAINTS);
-        }
-    }
-
-    /**
-     * Throws an {@code IllegalValueException} if {@code address} does not exist or is not valid.
-     *
-     * @throws IllegalValueException
-     */
-    private void checkAddressValid() throws IllegalValueException {
-        if (address == null) {
-            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Address.class.getSimpleName()));
-        }
-        if (!Address.isValidAddress(address)) {
-            throw new IllegalValueException(Address.MESSAGE_ADDRESS_CONSTRAINTS);
-        }
-    }
-
-    /**
-     * Throws an {@code IllegalValueException} if {@code loanStatus} does not exist or is not valid.
-     *
-     * @throws IllegalValueException
-     */
-    private void checkLoanStatusValid() throws IllegalValueException {
-        if (loanStatus == null) {
-            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT,
-                    LoanStatus.class.getSimpleName()));
-        }
-        if (!LoanStatus.isValidLoanStatus(loanStatus)) {
-            throw new IllegalValueException(LoanStatus.MESSAGE_LOANSTATUS_CONSTRAINTS);
-        }
-    }
-
-    /**
-     * Throws an {@code IllegalValueException} if {@code bike} does not exist or is not valid.
-     *
-     * @throws IllegalValueException
-     */
-    private void checkBikeValid() throws IllegalValueException {
-        if (bike == null) {
-            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Bike.class.getSimpleName()));
-        }
-        if (!Name.isValidName(bike)) {
-            throw new IllegalValueException(Name.MESSAGE_NAME_CONSTRAINTS);
-        }
-    }
-
-    /**
-     * Throws an {@code IllegalValueException} if {@code rate} does not exist or is not valid.
-     *
-     * @throws IllegalValueException
-     */
-    private void checkLoanRateValid() throws IllegalValueException {
-        if (rate == null) {
-            throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT,
-                    LoanRate.class.getSimpleName()));
-        }
-        if (!LoanRate.isValidRate(rate)) {
-            throw new IllegalValueException(LoanRate.MESSAGE_LOANRATE_CONSTRAINTS);
-        }
-    }
-
     /**
      * Throws an {@code IllegalValueException} if {@code startTime} does not exist or is not valid.
      *
@@ -280,28 +206,29 @@ public Loan toModelType() throws IllegalValueException {
             loanTags.add(tag.toModelType());
         }
 
-        checkNameValid();
+        checkFieldValid(name, Name.class, Name::isValidName, Name.MESSAGE_NAME_CONSTRAINTS);
         final Name modelName = new Name(name);
 
-        checkNricValid();
+        checkFieldValid(nric, Nric.class, Nric::isValidNric, Nric.MESSAGE_NRIC_CONSTRAINTS);
         final Nric modelNric = new Nric(nric);
 
-        checkPhoneValid();
+        checkFieldValid(phone, Phone.class, Phone::isValidPhone, Phone.MESSAGE_PHONE_CONSTRAINTS);
         final Phone modelPhone = new Phone(phone);
 
-        checkEmailValid();
+        checkFieldValid(email, Email.class, Email::isValidEmail, Email.MESSAGE_EMAIL_CONSTRAINTS);
         final Email modelEmail = new Email(email);
 
-        checkAddressValid();
+        checkFieldValid(address, Address.class, Address::isValidAddress, Address.MESSAGE_ADDRESS_CONSTRAINTS);
         final Address modelAddress = new Address(address);
 
-        checkLoanStatusValid();
+        checkFieldValid(loanStatus, LoanStatus.class, LoanStatus::isValidLoanStatus,
+            LoanStatus.MESSAGE_LOANSTATUS_CONSTRAINTS);
         final LoanStatus modelLoanStatus = LoanStatus.valueOf(loanStatus);
 
-        checkBikeValid();
+        checkFieldValid(bike, Bike.class, Name::isValidName, Name.MESSAGE_NAME_CONSTRAINTS);
         final Bike modelBike = new Bike(new Name(bike));
 
-        checkLoanRateValid();
+        checkFieldValid(rate, LoanRate.class, LoanRate::isValidRate, LoanRate.MESSAGE_LOANRATE_CONSTRAINTS);
         final LoanRate modelRate = new LoanRate(rate);
 
         checkLoanStartTimeValid();
@@ -310,7 +237,6 @@ public Loan toModelType() throws IllegalValueException {
         checkLoanEndTimeValid();
         final LoanTime modelEndTime = endTime == null ? null : new LoanTime(endTime);
 
-
         final Set<Tag> modelTags = new HashSet<>(loanTags);
         return new Loan(modelName,
                 modelNric,