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.

Sign up for GitHub

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

Jump to bottom

Fix: MFA Device recognition regression #145

Merged
merged 1 commit into from
Nov 17, 2023
Merged

Fix: MFA Device recognition regression #145

merged 1 commit into from
Nov 17, 2023

Conversation

pcmxgti
Copy link
Contributor

@pcmxgti pcmxgti commented Nov 15, 2023

No description provided.

@pcmxgti
Copy link
Contributor Author

pcmxgti commented Nov 15, 2023

@opis-mark This should fix #144 if you run Tokendito with --use-device-token twice in a row. Alternatively, you can pre-save your device token under okta_device_token in the config file. I'll do a bit more testing over the next few days before making this an official pull request.

@pcmxgti pcmxgti marked this pull request as ready for review November 16, 2023 01:57
@pcmxgti pcmxgti marked this pull request as draft November 16, 2023 01:57
@pcmxgti pcmxgti force-pushed the fix/unknown_device branch 2 times, most recently from e9777d4 to 3381625 Compare November 16, 2023 21:00
@ruhulio
Copy link
Contributor

ruhulio commented Nov 17, 2023

@pcmxgti Thanks for putting up this PR. After updating to 2.3.0 earlier today, I had noticed that the use_device_token read/write changes had been lost when merging with the OIE and cmd_interface changes. Let me know if there is anything I can help with around this.

@pcmxgti
Copy link
Contributor Author

pcmxgti commented Nov 17, 2023

@pcmxgti Thanks for putting up this PR. After updating to 2.3.0 earlier today, I had noticed that the use_device_token read/write changes had been lost when merging with the OIE and cmd_interface changes. Let me know if there is anything I can help with around this.

No worries! It turns out we also have to have a user agent that Okta likes for this to work correctly. We should be done with our testing shortly.

@pcmxgti pcmxgti force-pushed the fix/unknown_device branch 2 times, most recently from 0b225dd to 2c567b3 Compare November 17, 2023 19:03
@pcmxgti pcmxgti marked this pull request as ready for review November 17, 2023 19:05
ruhulio
ruhulio reviewed Nov 17, 2023
View reviewed changes
tests/unit/test_okta.py Outdated
@@ -585,7 +590,7 @@ def test_step_up_authenticate(mocker):
assert okta.step_up_authenticate(pytest_config, state_token) is False


def test_local_auth(mocker):
def test_local_authicate(mocker):
Copy link
Contributor

Choose a reason for hiding this comment

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

🐝 Spelling…

Suggested change
def test_local_authicate(mocker):
def test_local_authenticate(mocker):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! fixed.

@pcmxgti
Fix regression bug in MFA device recognition.
2469cfd 
* Fix regression bug that removed trusted device recongnition.
* Fix User-Agent field to work with Okta.
* Clean up cookie passing and handling, to simplify development.
@sevignyj sevignyj merged commit 5216955 into main Nov 17, 2023
23 checks passed
@pcmxgti pcmxgti deleted the fix/unknown_device branch November 20, 2023 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruhulio ruhulio ruhulio left review comments

@sevignyj sevignyj sevignyj approved these changes

Assignees

@sevignyj sevignyj

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pcmxgti @ruhulio @sevignyj