Skip to content

Allow pre-emptive specification of OTP with push#81

Closed
nneul wants to merge 1 commit intoonelogin:masterfrom
nneul:patch-2
Closed

Allow pre-emptive specification of OTP with push#81
nneul wants to merge 1 commit intoonelogin:masterfrom
nneul:patch-2

Conversation

@nneul
Copy link
Contributor

@nneul nneul commented Jan 21, 2023

If you provide OTP on CLI, this will include it with the first attempted push automatically, negating need to respond if it's already correct.

If you provide OTP on CLI, this will include it with the first attempted push automatically, negating need to respond if it's already correct.
@gkhaburzaniya-onelogin
Copy link
Contributor

It looks like a useful feature, but this code change makes the code misleading as it is nested within the OneLogin Protect branch. Currently the assumption is that if you're using OneLogin Protect, you want to use the push notifications, which it wouldn't make sense to take a CLI OTP for. We do not want to further obfuscate this code and add tech debt to this repo that will need to be changed later.

As this is a useful feature, we made our own stab at it here #82 so that the code isn't misleading. But we'd need more tests to make a change like this. We've put it in our issue tracker, but it may take a while, though if you need this fast you can try and write them.

Or a possible workaround is to use something other than OneLogin Protect for OTP.

@nneul
Copy link
Contributor Author

nneul commented Jan 24, 2023

Thanks. FYI, this submission was a direct result of trying to work around #80 - where I used to be able to simulate a push of the OTP. That suddenly stopped working.

I don't believe the particular OneLogin installation I'm working with has option for other token types. Will take a look at #82.

@nneul nneul closed this Aug 14, 2023
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.

2 participants