add new backend support for wott #1165

eugenosm · 2019-06-17T09:06:43Z

closes #1149
based on #1148 (PR #1162)

Introduce abstract Auth class, BasicAuth providing HTTP Authentication and its dummy subclass WoTTAuth. Move assetdir initialization to a separate function run by Flask.

Let Auth classes specify their config.

Rewrite auth settings update procedure.

Add NoAuth backend ("Disabled").

eugenosm · 2019-06-17T09:09:17Z

and it need to do something with permission to acces to credentials file

vpetersson · 2019-06-17T11:25:06Z

@a-martynovich Could you review the last part of this?

a-martynovich · 2019-06-17T17:57:09Z

auth.py

+
+        # self.settings['user'] = self.user
+        # self.settings['password'] = self.password
+        # self.settings['openpass'] = self.openpass


There're no settings, so update_settings shouldn't do anything.

a-martynovich · 2019-06-17T18:03:22Z

auth.py

+    def update_settings(self, current_password):
+        auth_backend = self.settings['auth_backend']
+        current_pass_correct = True if auth_backend == '' \
+            else self.settings.auth_backends[auth_backend].check_password(current_password)


@eugenosm Please rebase your branch on top of #1162 . In my PR this line looks different.

is_authorized -> is_authenticated, authorize -> authenticate_if_needed.

a-martynovich · 2019-06-19T17:51:18Z

@vpetersson what should happen if we can't load WoTT credentials? Hide the backend from the dropdown? Act as if auth is disabled? Show error when switching to WoTT backend (and disallow switching)?

vpetersson · 2019-06-19T18:49:56Z

@vpetersson what should happen if we can't load WoTT credentials? Hide the backend from the dropdown? Act as if auth is disabled? Show error when switching to WoTT backend (and disallow switching)?

That's an interesting use case.I don't think we should disable auth, as that's a potential attack vector. I'd suggest throwing a 50x maybe?

a-martynovich · 2019-06-20T01:43:46Z

I meant what if we can’t load wott credentials when we have other backend selected? Should we allow switching to wott backend in Settings (but show error after saving settings)? Or should we hide the option?

vpetersson · 2019-06-20T07:23:01Z

I meant what if we can’t load wott credentials when we have other backend selected? Should we allow switching to wott backend in Settings (but show error after saving settings)? Or should we hide the option?

Oh, i was purely talking about if WoTT was selected. If WoTT isn't selected, then it doesn't really matter. In that case we should probably just log it.

a-martynovich · 2019-06-20T07:24:05Z

I’m asking about th UI. Should we allow switching to wott backend in Settings (but show error after saving settings)? Or should we hide the option?

vpetersson · 2019-06-20T09:44:43Z

After a discussion on Slack.

If the file (normally /opt/wott/credentials/screenly.json unless overridden in config) is absent, prevent save and display “Can’t load /opt/wott/credentials/screenly.json”.
If the folder /opt/wott is absent, then assume the WoTT agent is not installed.

These are somewhat naive assumptions, but will do the trick for now.

korvyashka · 2019-06-20T10:07:41Z

auth.py

+from flask import request, Response
+
+WOTT_CREDENTIALS_PATH = '/opt/wott/credentials'
+WOTT_SCREENLY_CREDENTIAL_NAME = 'screenly_credentials'


If it is used only to be appended with ".json" - I guess it could be named WOTT_SCREENLY_CREDENTIAL_FILE

No, the logic implies that this name will be used in WoTT Credentials UI in the "Name" field.

korvyashka · 2019-06-20T10:27:59Z

auth.py

+                login_record = login_record.split(':', 1)
+                if len(login_record) == 2:
+                    self.user, password = login_record
+                    self.password = '' if password == '' else hashlib.sha256(password).hexdigest()


Seems password is stored in raw form in file - I'm not sure if it is fine, but I guess that is wott cred format.

Exactly so.

It's actually the way that the basic auth is done in Screenly OSE too. It's not good, but that's a problem for another day.

korvyashka · 2019-06-20T10:29:08Z

auth.py

+        return True
+
+
+class BasicAuth(Auth):


This seems to be the same code as in #1162 and I guess will be removed after requested by you rebase.

korvyashka · 2019-06-20T10:34:10Z

auth.py

+                    return True
+
+        self.user = ''
+        self.password = ''


Few minor comments here.

Can't it be done stateless, without need of storing user and password in the object? Just by reading them from cred during the check? Probably cached with cache decorators f.e?

I believe this function can be simplified by common practices:

Return early

Reverse if and return early

Moving these 2 statements to top of the func.

It can, but why? We already have username/password in settings (basic auth) loaded into memory from screenly.conf

Rewrite fetch_credentials. Hide WoTTAuth option if it can't load credentials at startup. Respond with 503 if auth backend raises ValueError.

a-martynovich · 2019-06-20T15:32:53Z

Follow-up changes:

WoTT option will be hidden if there's no /opt/wott directory.
Server will respond with 503 if credentials file suddenly disappears while WoTT backend is active.
Credentials are read at every request. There're few requests right now so it shouldn't be a problem (later we can add cache and stuff).

If credential file is missing when switching to WoTT backend in settings an error will be shown (it was already done by @eugenosm ).

a-martynovich and others added 19 commits June 14, 2019 16:34

Move auth code to auth.py.

6ad4a24

Introduce abstract Auth class, BasicAuth providing HTTP Authentication and its dummy subclass WoTTAuth. Move assetdir initialization to a separate function run by Flask.

Add docs to Auth class.

0a889d0

More docstrings and TODOs for WoTTAuth.

590a069

Configurable auth backend in ScreenlySettings.

0c1f18c

Let Auth classes specify their config.

Rename auth backend config groups.

5e3f0d3

Rewrite auth settings update procedure.

Fix pep8 warnings.

0531b6d

Auth backend select UI.

da05e74

Pass list of auth backends to settings template.

4784146

Add NoAuth backend ("Disabled").

Only show username/password settings for auth_basic.

5417d1c

Fix pep8 warnings.

4e93a37

Make auth backends UI-modular.

f297535

Fix auth backend switching error.

62109ff

add wott-credentials authority

0f403b0

merge with last Screenly#1148 updates (modular ui)

280f144

documentation added.

1cd9fc1

change two credentials records to one with value in form "user:password"

3b27831

change two credentials records to one with value in form "user:password"

602e03c

documentation update

5fac65a

codacy - corrections

6c8db20

pep8 correction

500a653

add condition if there is no login_credentials into fetch_credentials()

4da85ab

a-martynovich reviewed Jun 17, 2019

View reviewed changes

WoTTAuth: Properly split username:password.

6ccde00

a-martynovich force-pushed the 1149-add-new-backend-support-for-WoTT branch from 144c30c to 6ccde00 Compare June 18, 2019 18:23

a-martynovich and others added 3 commits June 19, 2019 00:58

Move current pass checking logic out of Auth to avoid duplication.

534bbcc

fix "no current password" when switch from disabled

a3b9362

Renaming Auth fields: name -> display_name, id -> name.

92b97fc

Renaming Auth methods:

31ce68e

is_authorized -> is_authenticated, authorize -> authenticate_if_needed.

korvyashka suggested changes Jun 20, 2019

View reviewed changes

a-martynovich mentioned this pull request Jun 20, 2019

Modular auth backend #1162

Closed

eugenosm and others added 4 commits June 20, 2019 20:44

add owner subfolder to wott credentials path

57b59ec

remove unused import

091c842

Rename credential name and key.

6387e1d

Rewrite fetch_credentials. Hide WoTTAuth option if it can't load credentials at startup. Respond with 503 if auth backend raises ValueError.

Rename credential name and key.

986d4ec

Rewrite fetch_credentials. Hide WoTTAuth option if it can't load credentials at startup. Respond with 503 if auth backend raises ValueError.

a-martynovich and others added 2 commits June 20, 2019 21:39

Fix pep8.

c39e8e1

update documentation

16ee757

vpetersson merged commit a4235a6 into Screenly:master Jun 21, 2019

Uh oh!

add new backend support for wott #1165

add new backend support for wott #1165

Uh oh!

Conversation

eugenosm commented Jun 17, 2019

Uh oh!

eugenosm commented Jun 17, 2019

Uh oh!

vpetersson commented Jun 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-martynovich commented Jun 19, 2019

Uh oh!

vpetersson commented Jun 19, 2019

Uh oh!

a-martynovich commented Jun 20, 2019

Uh oh!

vpetersson commented Jun 20, 2019

Uh oh!

a-martynovich commented Jun 20, 2019

Uh oh!

vpetersson commented Jun 20, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-martynovich commented Jun 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

a-martynovich commented Jun 20, 2019 •

edited

Loading