Password setup and bootstrap v4 migration #688

VakarisZ · 2020-06-16T10:47:54Z

What is this?

Fixes #596
Migrates UI to bootstrap v4.

Checklist

Have you added an explanation of what your changes do and why you'd like to include them?
Have you successfully tested your changes locally?
Is the TravisCI build passing?

Proof that it works

If applicable, add screenshots or log transcripts of the feature working

…not being sent

VakarisZ · 2020-06-16T10:51:31Z

monkey/monkey_island/cc/environment/test__init__.py

+            return []
+
+    # Username:test Password:test
+    CONFIG_WITH_CREDENTIALS = {


Making tests scattered all around the codebase might've been a mistake: where should common code between tests go?

Under common?

We have IslandTestCase for example which is common to a lot of tests and works fine ¯_(ツ)_/¯

ShayNehmad

Some minor comments, and a few usage questions:

Do we support changing the password? If so, how?
Do we support multiple users+password combos? If so, how?

The testing is very extensive and the design seems "lean and mean" to fit it! TDD paying dividends. 🤩

Also, like we've discussed, adding some short usage documentation with images/gifs will be useful (even on the open doc framework branch) , since this is the first screen that users are met with.

ShayNehmad · 2020-06-16T11:33:24Z

monkey/monkey_island/cc/environment/aws.py

+            # TODO change this to propper registration?
+            User(1, 'monkey', self.hash_secret(self._instance_id))


That's a good question.
On the one hand, forcing people to use that machine ID seems worse than them choosing their own password, on the other, might be a tiny bit more secure (the time in between setting up the machine and logging in), but that's really a stretch.

Let's delete this user and use the same login method across all our instances

Instance ID is public knowledge, right?

No, instance-id requires either running code on the machine or having access to the ec2 dashboard.

I think I've seen amazon support asking for it on a forum. Maybe it's not public knowledge, but it's not a secret either.

ShayNehmad · 2020-06-17T08:15:04Z

monkey/monkey_island/cc/environment/test__init__.py

+            return []
+
+    # Username:test Password:test
+    CONFIG_WITH_CREDENTIALS = {


We have IslandTestCase for example which is common to a lot of tests and works fine ¯_(ツ)_/¯

monkey/monkey_island/cc/resources/auth/user_store.py

ShayNehmad · 2020-06-17T08:25:00Z

monkey/monkey_island/cc/ui/src/components/Main.js

+            default:
+              return page_component;


Why is there a default case on a boolean value?

On component init these are set to undefined. It becomes boolean once the component gets response from server.

monkey/monkey_island/cc/ui/src/components/SideNavComponent.js

ShayNehmad · 2020-06-17T09:48:37Z

monkey/monkey_island/cc/environment/environment_config.py

+
+    @staticmethod
+    def get_config_file_path() -> str:
+        return os.path.join(MONKEY_ISLAND_ABS_PATH, 'cc/server_config.json')


This is a bug, should be os.path.join(MONKEY_ISLAND_ABS_PATH, 'cc', 'server_config.json')...

Or even better change MONKEY_ISLAND_ABS_PATH from a string to a Path (from pathlib) and then use

MONKEY_ISLAND_ABS_PATH.joinpath("cc").joinpath("server_config.json")

Not a bug, but I agree.

ShayNehmad · 2020-06-17T09:54:25Z

monkey/monkey_island/cc/environment/test_aws.py

@@ -7,7 +8,7 @@

 class TestAwsEnvironment(IslandTestCase):


We're changing the AWS env behaviour to be default, so this test might become redundant - if so delete it : )

ShayNehmad · 2020-06-17T09:55:42Z

monkey/monkey_island/cc/environment/test_environment_config.py

+        if platform.system() == "Windows":
+            server_file_path = MONKEY_ISLAND_ABS_PATH + "\cc/server_config.json"
+        else:
+            server_file_path = MONKEY_ISLAND_ABS_PATH + "/cc/server_config.json"
+        self.assertEqual(EnvironmentConfig.get_config_file_path(), server_file_path)


Path equality should be done with pathlib and not strings

I disagree: if test code is exactly the same as production then we'll get exactly the same results. This tests if we use pathlib correctly and if it's interface didn't change.

ShayNehmad · 2020-06-17T09:57:16Z

monkey/monkey_island/cc/resources/auth/auth_user.py

+
+class User(object):
+    def __init__(self, user_id, username, secret):
+        self.id = user_id


Are we ever using the UserId?

No, but I think JWT interface requires it and that's why it was added in the first place.

ShayNehmad · 2020-06-17T09:57:31Z

monkey/monkey_island/cc/environment/user_creds.py

+        return cred_dict
+
+    def to_auth_user(self) -> User:
+        return User(1, self.username, self.password_hash)


Why is the UserId const 1 ?

VakarisZ · 2020-06-19T13:17:57Z

Migrating to bootstrap v4 I had to use json-schema-b4 fork, that uses bootstrap v4 with font awesome. I couldn't find a way to include all css of font awesome through react-font-awesome package nor through font-awesome npm package, that's why I had to download the whole font awesome files and include them into the project manually. We could do CDN but I'd rather keep files local. We could also make these files into a fork/submodule?

VakarisZ · 2020-06-19T13:25:00Z

Do we support changing the password? If so, how?

Do we support multiple users+password combos? If so, how?
Also, like we've discussed, adding some short usage documentation with images/gifs will be useful (even on the open doc framework branch) , since this is the first screen that users are met with.

Yes. The exactly same way we did before: trough manual hash computation :D Or even better way available now: delete credentials and re-run island :)
No
Registration process should be intuitive. What's not intuitive is how to change credentials and that should be covered in docs for sure.

ShayNehmad · 2020-06-19T15:39:48Z

Migrating to bootstrap v4 I had to use json-schema-b4 fork, that uses bootstrap v4 with font awesome. I couldn't find a way to include all css of font awesome through react-font-awesome package nor through font-awesome npm package, that's why I had to download the whole font awesome files and include them into the project manually. We could do CDN but I'd rather keep files local. We could also make these files into a fork/submodule?

We can't do a CDN. No need to fork fontawesome (we're not planning on changing it). What's not working with npm install --save @fortawesome/fontawesome-free?

# Conflicts: # monkey/monkey_island/cc/resources/netmap.py # monkey/monkey_island/cc/ui/src/components/pages/RunMonkeyPage.js # monkey/monkey_island/cc/ui/src/components/report-components/security/ScannedServers.js

VakarisZ · 2020-06-22T11:35:43Z

We can't do a CDN. No need to fork fontawesome (we're not planning on changing it). What's not working with npm install --save @fortawesome/fontawesome-free?

I changed it by removing a big portion of unused code.
Icons are not being displayed, because react-json-schema-bs4 expects styles from all.css that are not included in @fortawesome/fontawesome-free

…path library in environment_config.py

…log is present

codecov · 2020-06-23T09:51:32Z

Codecov Report

Merging #688 into develop will increase coverage by 1.82%.
The diff coverage is 89.65%.

@@             Coverage Diff             @@
##           develop     #688      +/-   ##
===========================================
+ Coverage    58.06%   59.89%   +1.82%     
===========================================
  Files          139      147       +8     
  Lines         4483     4787     +304     
===========================================
+ Hits          2603     2867     +264     
- Misses        1880     1920      +40

Impacted Files	Coverage Δ
...em_info/windows_cred_collector/pypykatz_handler.py	`78.00% <ø> (ø)`
...fo/windows_cred_collector/test_pypykatz_handler.py	`100.00% <ø> (ø)`
monkey/monkey_island/cc/environment/password.py	`62.50% <25.00%> (-20.84%)`	⬇️
monkey/monkey_island/cc/services/config.py	`28.57% <33.33%> (ø)`
monkey/monkey_island/cc/resources/auth/auth.py	`48.14% <40.00%> (ø)`
monkey/monkey_island/cc/environment/aws.py	`52.63% <42.85%> (-47.37%)`	⬇️
monkey/monkey_island/cc/environment/standard.py	`87.50% <66.66%> (+1.78%)`	⬆️
monkey/monkey_island/cc/models/__init__.py	`90.90% <66.66%> (ø)`
...key_island/cc/environment/environment_singleton.py	`71.87% <71.87%> (ø)`
monkey/monkey_island/cc/environment/__init__.py	`77.90% <82.69%> (+4.83%)`	⬆️
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0a5c9e...8a31ff2. Read the comment docs.

monkey/monkey_island/cc/ui/src/components/pages/RegisterPage.js

ShayNehmad · 2020-06-23T12:56:36Z

monkey/monkey_island/cc/ui/src/components/pages/RegisterPage.js

+                      Lets Go!
+                    </Button>
+                    <a href='#' onClick={this.setNoAuth} className={'no-auth-link'}>
+                      I want anyone to access the island


perhaps change this to a button as well

Nahh, this should be a link with small text that no one clicks :)

# Conflicts: # .travis.yml

Will possibly help debug client issues with registration in future.

…setup

…into password_setup

…setup

… in text, because typographically it's not an apostrophe

VakarisZ added 12 commits June 10, 2020 11:52

Password setup: Backed environment changes and unit tests

18dec0c

Backend refactoring: server environment and authentication

c8cf7d5

Password setup - backend fixes

889bf35

Password setup - password environment by default

b419ed5

Refactored side navigation into a separate component

1b98080

Migration to bootstrap v4 started, Registration UI finished

ac83d7f

Skip performance tests that actually run monkeys by default

39231ab

Fixed registration bugs: empty password gets registered, credentials …

5996569

…not being sent

Reverted accidental test credential commit

947f9d2

Added server_config to gitignore

2601e27

Removed server_config from tracking

a5b1ac2

Finished "no password" environment option

ce5e415

VakarisZ commented Jun 16, 2020

View reviewed changes

ShayNehmad suggested changes Jun 17, 2020

View reviewed changes

VakarisZ added 3 commits June 19, 2020 15:35

Migration to bootstrap v4 changes

a1da0e3

Changed login page to match the style of registration page

1cc5310

Reverted server_config.json that contains test credentials

8af4246

VakarisZ changed the title ~~Password setup~~ Password setup and bootstrap v4 migration Jun 19, 2020

VakarisZ added 6 commits June 22, 2020 09:56

Merge remote-tracking branch 'upstream/develop' into password_setup

4a082fb

# Conflicts: # monkey/monkey_island/cc/resources/netmap.py # monkey/monkey_island/cc/ui/src/components/pages/RunMonkeyPage.js # monkey/monkey_island/cc/ui/src/components/report-components/security/ScannedServers.js

Bugfix: Include statement of absent PTH map removed

a093cb0

Bugfix: bad import path fix

85275f6

Added registration to aws, small typo fixes

8c428aa

Refactored try_add_user and needs_registration to avoid code duplication

78cf0b5

Rolled back merge changes that broke map page UI

79bf05d

VakarisZ added 2 commits June 22, 2020 15:17

Small improvements: aws test removed and path parsing changed to use …

90b05a1

…path library in environment_config.py

Lowered the amount of python lint warnings with small style improvements

9b5029a

VakarisZ added 6 commits June 23, 2020 10:32

JS minor quotes refactor

be51de2

Merge remote-tracking branch 'upstream/develop' into password_setup

0be1b3a

Bugfix: PreviewPane.js log download button no longer enabled when no …

bd537fa

…log is present

Changed testing documentation that relates to password protection

30fde6d

Linting fixes

4c8c423

More quote fixes, increased JS warnings to 500

09b8239

Missing svg image added, registration error overlap fixed

44c8ffd

ShayNehmad suggested changes Jun 23, 2020

View reviewed changes

VakarisZ and others added 12 commits June 23, 2020 16:31

Run monkey page improvement, small style and attack report fixes

37f356a

UI fixes for registration and login pages and zero trust report

468bd7d

Merge remote-tracking branch 'upstream/develop' into password_setup

7462c1c

# Conflicts: # .travis.yml

Small typo in register page, submit on enter fixed in auth pages

e58cdee

Adding some server logs re: registration

da58ea7

Will possibly help debug client issues with registration in future.

Redundant failed login attempts with no auth credentials fixed

d22e784

Merge remote-tracking branch 'upstream/password_setup' into password_…

938a5e2

…setup

Fix some whitespace issues in the events modal

27c50fd

Merge branch 'password_setup' of https://github.com/guardicore/monkey …

4c6f2ef

…into password_setup

Couple of PEP8 issues related to password creation files fixed

4e9a749

Merge remote-tracking branch 'upstream/password_setup' into password_…

edfbf37

…setup

Disabled no-unescaped-entities, because it throws warnings on ' usage…

8a31ff2

… in text, because typographically it's not an apostrophe

VakarisZ merged commit 6cc4c85 into develop Jun 25, 2020

VakarisZ deleted the password_setup branch December 8, 2020 11:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Password setup and bootstrap v4 migration #688

Password setup and bootstrap v4 migration #688

VakarisZ commented Jun 16, 2020

VakarisZ Jun 16, 2020

acepace Jun 16, 2020

ShayNehmad Jun 17, 2020

ShayNehmad left a comment

ShayNehmad Jun 16, 2020

VakarisZ Jun 19, 2020

acepace Jun 20, 2020

VakarisZ Jun 22, 2020

ShayNehmad Jun 17, 2020

ShayNehmad Jun 17, 2020

VakarisZ Jun 22, 2020

ShayNehmad Jun 17, 2020

VakarisZ Jun 22, 2020

ShayNehmad Jun 17, 2020

VakarisZ Jun 22, 2020

ShayNehmad Jun 17, 2020

VakarisZ Jun 22, 2020

ShayNehmad Jun 17, 2020

VakarisZ Jun 22, 2020 •

edited

Loading

ShayNehmad Jun 17, 2020

VakarisZ commented Jun 19, 2020

VakarisZ commented Jun 19, 2020

ShayNehmad commented Jun 19, 2020

VakarisZ commented Jun 22, 2020

codecov bot commented Jun 23, 2020 •

edited

Loading

ShayNehmad Jun 23, 2020

VakarisZ Jun 25, 2020

		# TODO change this to propper registration?
		User(1, 'monkey', self.hash_secret(self._instance_id))

Password setup and bootstrap v4 migration #688

Password setup and bootstrap v4 migration #688

Conversation

VakarisZ commented Jun 16, 2020

What is this?

Checklist

Proof that it works

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShayNehmad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VakarisZ Jun 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VakarisZ commented Jun 19, 2020

VakarisZ commented Jun 19, 2020

ShayNehmad commented Jun 19, 2020

VakarisZ commented Jun 22, 2020

codecov bot commented Jun 23, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VakarisZ Jun 22, 2020 •

edited

Loading

codecov bot commented Jun 23, 2020 •

edited

Loading