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

WebAuth Client MVP #933

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

Benjamin-Philip
Copy link
Contributor

@Benjamin-Philip Benjamin-Philip commented Dec 14, 2021

This is some code that should work for the WebAuth flow in theory.
Unfortunately, I have no way of testing, so this code may not actually
work.

This commit adds a experimental web auth flow task.
@Benjamin-Philip
Copy link
Contributor Author

As of right now, there are no tests due to the following issues:

  • I need to get mix task output in order to submit the user code
  • I need to mock a logged in user at /login/web_auth/submit

@ericmj
Copy link
Member

ericmj commented Dec 14, 2021

I need to get mix task output in order to submit the user code

All output is sent to the current test process. You can assert on it like this: https://github.com/hexpm/hex/blob/main/test/mix/tasks/hex.info_test.exs#L18.

I need to mock a logged in user at /login/web_auth/submit

If you need an endpoint on hexpm that does this you can add it here: https://github.com/hexpm/hexpm/blob/main/lib/hexpm_web/router.ex#L279-L298.

@Benjamin-Philip
Copy link
Contributor Author

If you need an endpoint on hexpm that does this you can add it here:

Are you suggesting that I call the WebAuth.submit function from the TestController with some random user?

@ericmj
Copy link
Member

ericmj commented Dec 14, 2021

Are you suggesting that I call the WebAuth.submit function from the TestController with some random user?

I am not sure what you need to do, but if you need a user to perform a webauth submit in a test scenario that could be one way of doing it.

@Benjamin-Philip Benjamin-Philip marked this pull request as ready for review December 22, 2021 10:13
@Benjamin-Philip
Copy link
Contributor Author

Benjamin-Philip commented Dec 22, 2021

@ericmj and @wojtekmach , I have got a working task ready. Here's a video showing the task:

hex_web_auth.mp4

Note: I think the frame rate is slow again, but now I think it's my screen recorder's fault.

@Benjamin-Philip
Copy link
Contributor Author

@ericmj, @wojtekmach Ping.

lib/hex/api/web_auth.ex Outdated Show resolved Hide resolved
lib/hex/api/web_auth.ex Outdated Show resolved Hide resolved
lib/hex/api/web_auth.ex Outdated Show resolved Hide resolved
lib/mix/tasks/hex.user.ex Outdated Show resolved Hide resolved
Process.sleep(100)

task.pid
|> get_user_code
Copy link
Member

Choose a reason for hiding this comment

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

You can use assert_received {:mix_shell, :info, [message]} to retrieve the test output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test output is actually sent to the Task process, so I can't use assert_received.

The reason I need to run the task in an other process is because, I need to extract the user_code from the test output, and then submit the user_code to hexpm for the web auth task to access the key.

Copy link
Member

Choose a reason for hiding this comment

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

How about submitting the auth request in another process instead. Would that make it easier?

Copy link
Contributor Author

@Benjamin-Philip Benjamin-Philip Feb 9, 2022

Choose a reason for hiding this comment

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

That actually would. I did not think of that.
Will implement when free.

There is no real difference between submitting the user code to hexpm in a seperate process and submitting in the test process.

This is because I can't access the task process's mailbox like normal, and I have to use Process.info to access its messages. See the get_user_code anonymous function for more info.

What would help is some way to "echo" messages sent to the task process to the test process, in which case I can just assert_recieved. However, I think this would bring more complexity than there is already existing.

lib/hex/api/web_auth.ex Outdated Show resolved Hide resolved
We don't want to try send an access_key request more than once per
second. Therefore, this commit adds support for a timeout after a
request, which will be used if a request completes in less than a
second.
@Benjamin-Philip
Copy link
Contributor Author

Benjamin-Philip commented Feb 11, 2022

I noticed that you ran the CI on the PR. The tests will fail until the backend PR has been merged.

If you would like to run the tests, I suggest that you checkout the PRs locally.

@ericmj ericmj force-pushed the main branch 2 times, most recently from 24c6c48 to 0edaffc Compare February 2, 2023 01:32
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