Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic Authentication #217

Merged
merged 15 commits into from Apr 21, 2020
Merged

Basic Authentication #217

merged 15 commits into from Apr 21, 2020

Conversation

stefanm8
Copy link
Contributor

This PR refers to #95

Is only the backend part

@stefanm8 stefanm8 changed the title Basic Authentication Backend Basic Authentication Apr 12, 2020
@stefanm8
Copy link
Contributor Author

stefanm8 commented Apr 12, 2020

I've included in this PR the frontend as well for this, currently there is no persistence of session. The way I see it, on the backend part expiry time must be added to token, and on the frontend save that token.

I'm not sure if the design aligns with the current design but its a start, and also I'm not a user of vue, let me know if I missed something.

router.push("/dashboard")
})
.catch(function() {
commit("app/SET_SNACKBAR", { text: "Invalid credentials", color: "red" }, { root: true })
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was supposed to bring up a snack, it doesn't and. not sure why this happens.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's okay, we can take a look.

@stefanm8 stefanm8 marked this pull request as draft April 15, 2020 09:50
@stefanm8 stefanm8 requested a review from kevgliss April 15, 2020 13:11
@lgtm-com
Copy link

lgtm-com bot commented Apr 15, 2020

This pull request introduces 1 alert when merging c302630 into 03e2a9c - view on LGTM.com

new alerts:

  • 1 for 'import *' may pollute namespace

.vscode/settings.json Outdated Show resolved Hide resolved
@@ -20,4 +20,18 @@ instance.interceptors.request.use(
}
)

instance.interceptors.response.use(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today, we check for auth on route load (instead of 401) and then have a timer telling a user to refresh before their token expires.
See here:
https://github.com/Netflix/dispatch/blob/develop/src/dispatch/static/dispatch/src/router/index.js#L137
and
https://github.com/Netflix/dispatch/blob/develop/src/dispatch/static/dispatch/src/auth/store.js#L31

I'm not sure how that fits in with the login feature? Perhaps we set the same expiration but send users to /login when basic authentication is enabled?

We will also need a way to tell the frontend to use PKCE or basic authentication via the DISPATCH_AUTHENTICATION_PROVIDER_SLUG here:

https://github.com/Netflix/dispatch/blob/develop/src/dispatch/static/dispatch/src/router/index.js#L31

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the auth check, in the router/index.js although I kept the check on the responses so I can effectively logout the user, this should not interact with any other auth provider, unless there are 401 returned (which is not a bad intent to logout user if 401 is received)

This should not interact with the current login feature. Exactly when no auth provider slug is provided basic auth will be used and this handler will be accessed "loginBasic" from router/index.js.

I decided to let it empty and modify value of VUE_DISPATCH_AUTHENTICATION_PROVIDER_SLUG to set a default value in case no slug is present

@lgtm-com
Copy link

lgtm-com bot commented Apr 16, 2020

This pull request introduces 1 alert when merging 81675b6 into 18b9b63 - view on LGTM.com

new alerts:

  • 1 for 'import *' may pollute namespace

@stefanm8 stefanm8 marked this pull request as ready for review April 16, 2020 11:24
@@ -138,7 +152,7 @@ router.beforeEach((to, from, next) => {
if (authProviderSlug === "dispatch-auth-provider-pkce") {
loginwithPKCE(to, from, next)
} else {
next()
Copy link
Contributor

@kevgliss kevgliss Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to provide a dispatch-auth-provider-basic slug. There are users who don't wish to enable any kind of auth (they are handling it outside of dispatch). That said I'm okay making dispatch-auth-provider-basic the default here: https://github.com/Netflix/dispatch/blob/develop/src/dispatch/config.py#L75

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you are saying, I added to the config "dispatch-auth-provider-basic". I did not know this app is usable without any authentication.

@lgtm-com
Copy link

lgtm-com bot commented Apr 19, 2020

This pull request introduces 1 alert when merging 861399b into e975aa4 - view on LGTM.com

new alerts:

  • 1 for 'import *' may pollute namespace

@kevgliss
Copy link
Contributor

Awesome, this looks good. I'll merge this in and test it out with some other changes. I wanted to get a release out before merging this in, so thanks for hanging in there and making changes.

@kevgliss kevgliss merged commit 96948ae into Netflix:develop Apr 21, 2020
kevgliss added a commit that referenced this pull request May 18, 2020
* Basic Authentication  (#217)

Initial work on Basic Authentication.

* Task/active tasks (#240)

* Adjusting tabs so they render correctly.

* Only create tasks notifications for active incidents.

* Allows incidents to be filtered by tag. (#238)

* Allows incidents to be filtered by tag.

* Adds 'load more' item to find tags not returned in the first 5 results

* Feature/incident auto tagger (#233)

* Initial work on auto tagging incidents

* Initial work on decoupling plugins from creation flow (#223)

* Initial work on decoupling plugins from creation flow

* Small plugin fixes with alembic. (#242)

* Updates auth and incident cost docs (#245)

* Let's use Join Incident in all Slack buttons (#246)

* Moving models around such that they are available for use. (#249)

* Allows user auth to be disabled returning a default user. (#250)

* Making some minor fixes around basic auth (#241)

* Making some minor fixes around basic auth

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Minor edit (#251)

* Moving basic auth to an auth plugin (#252)

* Fixes incident types and priorities paths in the Web UI router config (#256)

* Fixes incident types and priorities paths in the Web UI

* Adds note

* Importing main instead of api directly (#258)

* Upgraded node version to 12.16.2 (#260)

Signed-off-by: Daniel Sutton <daniel@ducksecops.uk>

* Adds all plugin event API routes to the API router before adding them to the Web API framework (#261)

* Adds flags to plugins allowing them to specify if they are required (#259)

* Adds flags to plugins allowing them to specify if they can have more than one or are required for dispatch to work.

* Adds more timeline events (#262)

* Cleaning up old un-used commands (#263)

* Reads variables from .env file as well as envvars (#264)

* Refactors nlp logic in the route service to use the centralized nlp module (#265)

* Fixing an issue with database being in a bad state (#268) (#269)

* Fixes bug in the incident cost model (#274)

* Fixing an issue with database being in a bad state (#268)

* Fixes bug in the incident cost model (#273)

Co-authored-by: kevgliss <kevgliss@gmail.com>

* fix typo in setup.py (#275)

* Fixed empty enabled column in table 'plugin' (#270)

* Fixed empty enabled column in table 'plugin'

* no getattr

* Bugfix router login redirect (#277)

* Fixed empty enabled column in table 'plugin'

* no getattr

* the user should continue on

* Fixes metric query, and reduces bundle size by allowing for treeshaki… (#276)

* Fixes metric query, and reduces bundle size by allowing for treeshaking of lodash.

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Fixes plugins version (#278)

* Let's allow users to disable a plugin (#280)

* Correctly check the current revision (#281)

* Timeline event details (#282)

* Updates typescript

* Adds JSON-based, details column to the event data model

* Adds switch

* Using v-card so we can show all details

* Addresses comments

* Removes unused import

* Allowing resources to be optionally created (#279)

* Refactors incident table filter (#284)

* Adding default icon (#285)

* Initial work adding RBAC controls (#266)

* Adding basic RBAC controls

* Removing loading overlay (#286)

* Fetching current user info from server (#287)

* Fixing the generation of swagger documentation (#289)

* Disabling after hours until we figure out a caching fix. (#290)

* Fixes help links for multi-messages and adds task resolution directions. (#288)

* Fixes help links for multi-messages and adds task resolution directions.

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Removing dead code (#292)

* Moving notification caching to the database. (#291)

* Moving notification caching to the database

* removing un-needed code

* We return a user object from current_user instead of an email string (#293)

* Improves Dockerfile (#296)

* Adding Conference tests. (#267)

* `yield_fixture` is deprecated after pytest 3.0.

Plain old `fixture` is fine.

* [WIP] Adding Conference model and service tests.

* Use four spaces per indent in Python files.

* Add assertions and stub functions.

* Fix naming convention for conference tests.
* Remove 'name', as it's not present in the Conference model.

* Return a List of Conferences.

When asking for all the Conferences of a given resource type, it doesn't
make sense to ask for only one, or None (and throw an exception). It
seems more sensible to request the whole list, or even a paginated list.
Since this method isn't used anywhere in the code yet, we'll
just return all().

* Black formatting.

* Ensure our stub functions have args.

* Test retrieval of a List of Conferences.

* Add a test fixture to create multiple conferences.

* Class Meta above instance vars

* Incorporating suggested change

* Lowercase as suggested

* Implements test_conference_get_by_conference_id

* get_by_incident_id fails

* Implements test_conference_get_all

* Ensure we're testing the created Conference.

* Move ConferenceFactory later.

This way an IncidentFactory has been defined and is available.

* Give mocked Incident objects a fake ID.

* This test works fine now.

* Adds Join Incident button to stable incidents in daily summary (#298)

* Prevents user being redirected to /login on 401 requests for pkce flow (#300)

* Prevents user being redirected to /login on 401 requests for pkce flow

* Modifying incident get permissions

* changing from prophet to statsmodels (#297)

Co-authored-by: Shannon Morrison <shannonm@netflix.com>

* Fixes the dialog behavior of columns (#304)

* Fixes the dialog behavior of columns

* Removing un-related files

* Reporting incidents via Slack slash command (#294)

* #180 : initial commit - initiating incident from slack via slack commands

* Update src/dispatch/plugins/dispatch_slack/views.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/views.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/config.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/messaging.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/messaging.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/plugin.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/views.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/messaging.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* updates based on comments provided

* add report-incident to conversation enums.py

* fix issues after starting the application dispatch

* Delete launch.json

* Delete settings.json

* Revert "Delete launch.json"

This reverts commit 55e2c40

Co-authored-by: Vagharsh Kandilian <vagharsh.kandilian@disqo.com>
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Adds expressions to hybrid properties (#124)

* Adds expressions to hybrid properties

* Removing commend

* Fixes bugs in report incident Slack command (#305)

* Making flake8 happy (#307)

* Making flake8 happy

* More fixes

* MOAR fixes

* Making statsmodels a required package (#308)

* Making statsmodels a required package

* Minor fixes

* modifying launch (#309)

* Failing gracefully if there is insufficent data. (#310)

* Adjusting default landing page to admin UI dashboard. (#313)

* Fixes bug and typo in status notifications (#314)

* Fixes table loading state when API call fails (#315)

* Adds hostname and database name to confirmation (#316)

* Removing exception used for testing (#317)

* Closes the report incident modal after it's submitted (#318)

* Fixes an issue where the report incident modal is not closing after submitting

* Using private metadata field to store channel id

* Bugfix/search ui (#321)

* Initial search work

* Adding clickable items

* Adds as join incident button to the incident status page (#322)

* Adds as join incident button to the incident status page

* Using incident flow

* Fixes search/filter pagination reset (#323)

* Standarizes save/close button icons (#324)

* Using checkbox for table booleans (#325)

* Ensures all table actions are under row ellipsis menu (#326)

* fixing linting issue (#329)

* Add custom profile field mapping for slack to get Team, Department, W… (#327)

* Add custom profile field mapping for slack to get Team, Department, Weblink

* Update config names to address PR feedback

* Update docs with new config vars

* Revert "Update docs with new config vars"

This reverts commit 7889530.

* update docs and revert unwanted files

* remove unused import

* Adding a unique constraint on the dispatch user email (#331)

* Add author and author URL to zoom plugin to fix plugins page (#330)

Co-authored-by: Stefan Mihartescu <stefanmihartescu@gmail.com>
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
Co-authored-by: Daniel Sutton <daniel@ducksecops.uk>
Co-authored-by: batman <5187696+mlioo@users.noreply.github.com>
Co-authored-by: Forest Monsen <forest.monsen@gmail.com>
Co-authored-by: Shannon <snkilmartin@users.noreply.github.com>
Co-authored-by: Shannon Morrison <shannonm@netflix.com>
Co-authored-by: Vagharsh Kandilian <vagharsh@users.noreply.github.com>
Co-authored-by: Vagharsh Kandilian <vagharsh.kandilian@disqo.com>
Co-authored-by: Will Bengtson <william.bengtson@gmail.com>
kevgliss added a commit that referenced this pull request Jun 3, 2020
* Basic Authentication  (#217)

Initial work on Basic Authentication.

* Task/active tasks (#240)

* Adjusting tabs so they render correctly.

* Only create tasks notifications for active incidents.

* Allows incidents to be filtered by tag. (#238)

* Allows incidents to be filtered by tag.

* Adds 'load more' item to find tags not returned in the first 5 results

* Feature/incident auto tagger (#233)

* Initial work on auto tagging incidents

* Initial work on decoupling plugins from creation flow (#223)

* Initial work on decoupling plugins from creation flow

* Small plugin fixes with alembic. (#242)

* Updates auth and incident cost docs (#245)

* Let's use Join Incident in all Slack buttons (#246)

* Moving models around such that they are available for use. (#249)

* Allows user auth to be disabled returning a default user. (#250)

* Making some minor fixes around basic auth (#241)

* Making some minor fixes around basic auth

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Minor edit (#251)

* Moving basic auth to an auth plugin (#252)

* Fixes incident types and priorities paths in the Web UI router config (#256)

* Fixes incident types and priorities paths in the Web UI

* Adds note

* Importing main instead of api directly (#258)

* Upgraded node version to 12.16.2 (#260)

Signed-off-by: Daniel Sutton <daniel@ducksecops.uk>

* Adds all plugin event API routes to the API router before adding them to the Web API framework (#261)

* Adds flags to plugins allowing them to specify if they are required (#259)

* Adds flags to plugins allowing them to specify if they can have more than one or are required for dispatch to work.

* Adds more timeline events (#262)

* Cleaning up old un-used commands (#263)

* Reads variables from .env file as well as envvars (#264)

* Refactors nlp logic in the route service to use the centralized nlp module (#265)

* Fixing an issue with database being in a bad state (#268) (#269)

* Fixes bug in the incident cost model (#274)

* Fixing an issue with database being in a bad state (#268)

* Fixes bug in the incident cost model (#273)

Co-authored-by: kevgliss <kevgliss@gmail.com>

* fix typo in setup.py (#275)

* Fixed empty enabled column in table 'plugin' (#270)

* Fixed empty enabled column in table 'plugin'

* no getattr

* Bugfix router login redirect (#277)

* Fixed empty enabled column in table 'plugin'

* no getattr

* the user should continue on

* Fixes metric query, and reduces bundle size by allowing for treeshaki… (#276)

* Fixes metric query, and reduces bundle size by allowing for treeshaking of lodash.

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Fixes plugins version (#278)

* Let's allow users to disable a plugin (#280)

* Correctly check the current revision (#281)

* Timeline event details (#282)

* Updates typescript

* Adds JSON-based, details column to the event data model

* Adds switch

* Using v-card so we can show all details

* Addresses comments

* Removes unused import

* Allowing resources to be optionally created (#279)

* Refactors incident table filter (#284)

* Adding default icon (#285)

* Initial work adding RBAC controls (#266)

* Adding basic RBAC controls

* Removing loading overlay (#286)

* Fetching current user info from server (#287)

* Fixing the generation of swagger documentation (#289)

* Disabling after hours until we figure out a caching fix. (#290)

* Fixes help links for multi-messages and adds task resolution directions. (#288)

* Fixes help links for multi-messages and adds task resolution directions.

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Removing dead code (#292)

* Moving notification caching to the database. (#291)

* Moving notification caching to the database

* removing un-needed code

* We return a user object from current_user instead of an email string (#293)

* Improves Dockerfile (#296)

* Adding Conference tests. (#267)

* `yield_fixture` is deprecated after pytest 3.0.

Plain old `fixture` is fine.

* [WIP] Adding Conference model and service tests.

* Use four spaces per indent in Python files.

* Add assertions and stub functions.

* Fix naming convention for conference tests.
* Remove 'name', as it's not present in the Conference model.

* Return a List of Conferences.

When asking for all the Conferences of a given resource type, it doesn't
make sense to ask for only one, or None (and throw an exception). It
seems more sensible to request the whole list, or even a paginated list.
Since this method isn't used anywhere in the code yet, we'll
just return all().

* Black formatting.

* Ensure our stub functions have args.

* Test retrieval of a List of Conferences.

* Add a test fixture to create multiple conferences.

* Class Meta above instance vars

* Incorporating suggested change

* Lowercase as suggested

* Implements test_conference_get_by_conference_id

* get_by_incident_id fails

* Implements test_conference_get_all

* Ensure we're testing the created Conference.

* Move ConferenceFactory later.

This way an IncidentFactory has been defined and is available.

* Give mocked Incident objects a fake ID.

* This test works fine now.

* Adds Join Incident button to stable incidents in daily summary (#298)

* Prevents user being redirected to /login on 401 requests for pkce flow (#300)

* Prevents user being redirected to /login on 401 requests for pkce flow

* Modifying incident get permissions

* changing from prophet to statsmodels (#297)

Co-authored-by: Shannon Morrison <shannonm@netflix.com>

* Fixes the dialog behavior of columns (#304)

* Fixes the dialog behavior of columns

* Removing un-related files

* Reporting incidents via Slack slash command (#294)

* #180 : initial commit - initiating incident from slack via slack commands

* Update src/dispatch/plugins/dispatch_slack/views.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/views.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/config.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/messaging.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/messaging.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/plugin.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/views.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Update src/dispatch/plugins/dispatch_slack/messaging.py

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* updates based on comments provided

* add report-incident to conversation enums.py

* fix issues after starting the application dispatch

* Delete launch.json

* Delete settings.json

* Revert "Delete launch.json"

This reverts commit 55e2c40

Co-authored-by: Vagharsh Kandilian <vagharsh.kandilian@disqo.com>
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>

* Adds expressions to hybrid properties (#124)

* Adds expressions to hybrid properties

* Removing commend

* Fixes bugs in report incident Slack command (#305)

* Making flake8 happy (#307)

* Making flake8 happy

* More fixes

* MOAR fixes

* Making statsmodels a required package (#308)

* Making statsmodels a required package

* Minor fixes

* modifying launch (#309)

* Failing gracefully if there is insufficent data. (#310)

* Adjusting default landing page to admin UI dashboard. (#313)

* Fixes bug and typo in status notifications (#314)

* Fixes table loading state when API call fails (#315)

* Adds hostname and database name to confirmation (#316)

* Removing exception used for testing (#317)

* Closes the report incident modal after it's submitted (#318)

* Fixes an issue where the report incident modal is not closing after submitting

* Using private metadata field to store channel id

* Bugfix/search ui (#321)

* Initial search work

* Adding clickable items

* Adds as join incident button to the incident status page (#322)

* Adds as join incident button to the incident status page

* Using incident flow

* Fixes search/filter pagination reset (#323)

* Standarizes save/close button icons (#324)

* Using checkbox for table booleans (#325)

* Ensures all table actions are under row ellipsis menu (#326)

* fixing linting issue (#329)

* Add custom profile field mapping for slack to get Team, Department, W… (#327)

* Add custom profile field mapping for slack to get Team, Department, Weblink

* Update config names to address PR feedback

* Update docs with new config vars

* Revert "Update docs with new config vars"

This reverts commit 7889530.

* update docs and revert unwanted files

* remove unused import

* Adding a unique constraint on the dispatch user email (#331)

* Add author and author URL to zoom plugin to fix plugins page (#330)

Co-authored-by: Stefan Mihartescu <stefanmihartescu@gmail.com>
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
Co-authored-by: Daniel Sutton <daniel@ducksecops.uk>
Co-authored-by: batman <5187696+mlioo@users.noreply.github.com>
Co-authored-by: Forest Monsen <forest.monsen@gmail.com>
Co-authored-by: Shannon <snkilmartin@users.noreply.github.com>
Co-authored-by: Shannon Morrison <shannonm@netflix.com>
Co-authored-by: Vagharsh Kandilian <vagharsh@users.noreply.github.com>
Co-authored-by: Vagharsh Kandilian <vagharsh.kandilian@disqo.com>
Co-authored-by: Will Bengtson <william.bengtson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants