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

fix: cypress test for GitHub login do not try to revalidate the app #35

Merged

Conversation

gentlementlegen
Copy link
Member

@gentlementlegen gentlementlegen commented Mar 20, 2024

This PR aims to solve issues encountered when running Cypress for the login test step.

The main issue is that GitHub rate limits users on how many times they can hit the login endpoint. If too many tests are run, the redirect doesn't happen on login and instead asks the user to revalidate the app. A case was made for this workflow, but after testing more thoroughly I saw that GitHub has a hard limit as shown in the screenshot
image

I don't think there is any workaround on this, since we have no control other those limits. It is not possible to fake an OAuth except if we run all locally (very unlikely scenario). I think this would not happen often if users try locally with their own account, since the Cypress tests will run once in a while and not every 2 seconds. So this scenario should (hopefully) be unlikely to happen. Still, few measures were taken:

  • enabling Cypress videos to help debugging
  • add an explicit comment and the necessary code to solve the issue when the rate limit is exceeded
  • updated Cypress to the latest version
  • changed Cypress trigger to be pushes to the development branch as I know @rndquu is eager to remove the pull_request_target events

@ubiquity-os-deployer
Copy link

ubiquity-os-deployer bot commented Mar 20, 2024

@0x4007
Copy link
Member

0x4007 commented Mar 20, 2024

I don't think there is any workaround on this, since we have no control other those limits

If you want to over engineer this we can use free proxies like from spys.one

@gentlementlegen
Copy link
Member Author

I don't think there is any workaround on this, since we have no control other those limits

If you want to over engineer this we can use free proxies like from spys.one

I am not even sure it would fix the problem since it is based on the amounts of logins you do with a specific account, it maybe does not even depend on an IP address, but could be tried.

@0x4007
Copy link
Member

0x4007 commented Mar 22, 2024

I don't think there is any workaround on this, since we have no control other those limits

If you want to over engineer this we can use free proxies like from spys.one

I am not even sure it would fix the problem since it is based on the amounts of logins you do with a specific account, it maybe does not even depend on an IP address, but could be tried.

Is it worth it to rotate between fake accounts? It's pretty easy to make new GitHub accounts.

@gentlementlegen
Copy link
Member Author

I don't think there is any workaround on this, since we have no control other those limits

If you want to over engineer this we can use free proxies like from spys.one

I am not even sure it would fix the problem since it is based on the amounts of logins you do with a specific account, it maybe does not even depend on an IP address, but could be tried.

Is it worth it to rotate between fake accounts? It's pretty easy to make new GitHub accounts.

I also thought about this. Probably can be achieved, it would require creating a new email each test with something like https://ethereal.email/ to do so. Just thought it'd be overkill at least for now since this should not manifest often.

@0x4007
Copy link
Member

0x4007 commented Mar 22, 2024

With Cloudflare its easy to make catch-all emails. This way we can have a single domain and then dynamically enter in any email address we want, while all of the messages forwarding to a single inbox.

@gentlementlegen
Copy link
Member Author

Sounds good. But we should make the whole workflow of registering accounts, which includes validating email etc. Might want a dedicated task for this. That PR should be enough of a fix for the current problem.
life-fixing 1

@0x4007
Copy link
Member

0x4007 commented Mar 22, 2024

we should make the whole workflow of registering accounts

We just care about GitHub accounts not emails though. I don't think we need to make a big project out of it?

For identities in the DevPool we just need GitHub accounts and those associated with wallet addresses.

@gentlementlegen
Copy link
Member Author

But I am confused how can you make GitHub accounts without email? Or maybe you meant having a pool of emails and rotate between them?

@0x4007
Copy link
Member

0x4007 commented Mar 22, 2024

Or maybe you meant having a pool of emails and rotate between them?

Yes but that's interesting if you're proposing to generate random emails and then actually logging into some shared inbox and clicking the link. Its theoretically possible, especially if that inbox forwards emails to some service that we can easily access the emails from some restful API.

There are also some automations for Gmail that can automatically click on registration links etc. I used to have some old AppsScript that would automatically unsubscribe from newsletters by clicking any links that regex match unsubscribe etc

@gentlementlegen
Copy link
Member Author

Or maybe you meant having a pool of emails and rotate between them?

Yes but that's interesting if you're proposing to generate random emails and then actually logging into some shared inbox and clicking the link. Its theoretically possible, especially if that inbox forwards emails to some service that we can easily access the emails from some restful API.

There are also some automations for Gmail that can automatically click on registration links etc. I used to have some old AppsScript that would automatically unsubscribe from newsletters by clicking any links that regex match unsubscribe etc

Nodemailer is also a good tool for that, used it before for similar purposes. I have to look closer to what Cloudflare can offer on that regard, maybe that's our best solution. We might wanna merge that for now to fix the current test, and dedicate a task for more advanced testing!

@0x4007
Copy link
Member

0x4007 commented Mar 22, 2024

You can merge when you think this is good to go.

@gentlementlegen gentlementlegen merged commit d413ea3 into ubiquity:development Mar 22, 2024
1 check passed
@gentlementlegen gentlementlegen deleted the fix/cypress-tests branch March 22, 2024 11:21
ghost pushed a commit to ariesgun/work.ubq.fi that referenced this pull request Oct 13, 2024
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