permitio / permit-cli

A command line utility from Permit.io to work with everything IAM and Authorization. A one-stop-shop to manage all your Authorization tools (OPA, OpenFGA, Cedar, OPAL, AVP...) as well as the Permit Service.
21 stars 27 forks source link

Refactoring Login Flow and Handling Errors #13

Open gemanor opened 4 weeks ago

gemanor commented 4 weeks ago

Before attempting this issue, review the code, try the feature, and understand the requirements. Attemptemt requires a detailed design proposal for the refactoring work. There’s no first come, first serve for this bounty; the maintenance team will select the best design proposal.

ATM, the login flow and select environment in Permit is written in a long spaghetti component that needs to be broken into the following pure components:

The login flow should be kept as it is today. Start with the login flow and end with storing the API key from the selected environment.

Acceptance criteria:

gemanor commented 4 weeks ago

/bounty 300

algora-pbc[bot] commented 4 weeks ago

💎 $300 bounty • Permit.io

Steps to solve:

  1. Start working: Comment /attempt #13 with your implementation plan
  2. Submit work: Create a pull request including /claim #13 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to permitio/permit-cli!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🔴 @vikashsprem Oct 28, 2024, 3:56:23 PM WIP
🟢 @varshith257 Oct 28, 2024, 3:58:54 PM WIP
🔴 @Rutik7066 Nov 6, 2024, 3:28:54 PM WIP
🟢 @hoklims Nov 20, 2024, 9:58:38 AM #33
🟢 @35C4n0r #28
vikashsprem commented 4 weeks ago

/attempt #13

Algora profile Completed bounties Tech Active attempts Options
@vikashsprem 2 bounties from 2 projects
TypeScript, JavaScript,
Go & more
Cancel attempt
varshith257 commented 4 weeks ago

/attempt #13

Algora profile Completed bounties Tech Active attempts Options
@varshith257 17 bounties from 9 projects
Go, Rust,
Scala & more
Cancel attempt
35C4n0r commented 3 weeks ago

@gemanor Here is my proposal

Hello Gabriel, this is my proposal for https://github.com/permitio/permit-cli/issues/13,

We break the current login.tsx in to 6 components, namely:

  • LoginFlow - Handles opening the browser for authentication or validating the API key.
  • EnvironmentSelection - Selects the organization, project, and environment.
  • SelectWorkspace - Selects the workspace if needed.
  • SelectProject - Selects the project.
  • SelectEnvironment - Selects the environment.
  • StoreKeychain - Stores the key in the keychain.

The new login.tsx component would look something like this:

  return (
    <>
      {state === 'login' && (
        <LoginFlow 
          apiKey={key} 
          onSuccess={handleLoginSuccess} 
          onError={handleLoginError} 
        />
      )}

      {state === 'selectEnvironment' && accessToken && cookie && (
        <EnvironmentSelection 
          accessToken={accessToken} 
          cookie={cookie} 
          workspace={workspace} 
          onComplete={handleEnvironmentSelectionComplete} 
        />
      )}

      {state === 'done' && secret && (
        <>
          <StoreKeychain secret={secret} />
          <Text>Login complete and API key stored!</Text>
        </>
      )}

      {state === 'done' && authError && (
        <Text color="red">{authError}</Text>
      )}
    </>
  );

I also found a bug while working on Windows,

netsh interface ipv4 show excludedportrange protocol=tcp

Protocol tcp Port Exclusion Ranges

Start Port    End Port
----------    --------
     50000       50059     *
     51742       51841
     51842       51941
     55869       55968
     55969       56068
     56069       56168
     56169       56268
     56269       56368
     56369       56468
     56469       56568
     62092       62191
     62192       62291
     62292       62391
     62392       62491
     62492       62591
     62592       62691
     62696       62795

* - Administered port exclusions.

We currently use 62419 in browser auth0 authentication (redirection_uri) which leads to this error

⠋ Logging in...
node:events:497
      throw er; // Unhandled 'error' event
      ^

Error: listen EACCES: permission denied ::1:62419
    at Server.setupListenHandle [as _listen2] (node:net:1881:21)
    at listenInCluster (node:net:1946:12)
    at GetAddrInfoReqWrap.doListen [as callback] (node:net:2116:7)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:111:8)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1925:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EACCES',
  errno: -4092,
  syscall: 'listen',
  address: '::1',
  port: 62419
}

I'll also include a solution in my PR to solve this issue. My proposal is to choose another port, like 62692, for Windows. This will also require you to add this URL to the whitelist in auth0.

I've played around with the code I can ensure a quick turnaround time for this project.

gemanor commented 3 weeks ago

@35C4n0r the general proposal looks good to me, I just wonder why's the store keychain need to be a component and not a hook/provider/etc.? Do you think there's a need with UI component of it?

35C4n0r commented 3 weeks ago

@gemanor you're right; I'll change that to either a hook or a utility function. I also plan to break down the different API calls in login.tsx into separate hooks based on their types. For example:

useOrgs: will handle methods related to orgs API calls. etc.

gemanor commented 3 weeks ago

@35C4n0r looks good to me. Please consider all the functions to be pure in case their not need to be part of the component state.

algora-pbc[bot] commented 2 weeks ago

💡 @35C4n0r submitted a pull request that claims the bounty. You can visit your bounty board to reward.

Rutik7066 commented 2 weeks ago

/attempt #13

Algora profile Completed bounties Tech Active attempts Options
@Rutik7066 10 bounties from 7 projects
Go, Rust,
MDX & more
﹟18
Cancel attempt
Rutik7066 commented 2 weeks ago

@35C4n0r @gemanor I’m on Windows, and this isn’t a bug; it happens when other services occupy the port. I've encountered this too. Using Ctrl + C to stop the CLI app closes it, but it takes 3 to 4 seconds to clean up processes like the Node server, causing the error.

I also found a bug while working on Windows,

netsh interface ipv4 show excludedportrange protocol=tcp

Protocol tcp Port Exclusion Ranges

Start Port    End Port
----------    --------
     50000       50059     *
     51742       51841
     51842       51941
     55869       55968
     55969       56068
     56069       56168
     56169       56268
     56269       56368
     56369       56468
     56469       56568
     62092       62191
     62192       62291
     62292       62391
     62392       62491
     62492       62591
     62592       62691
     62696       62795

* - Administered port exclusions.

We currently use 62419 in browser auth0 authentication (redirection_uri) which leads to this error

⠋ Logging in...
node:events:497
      throw er; // Unhandled 'error' event
      ^

Error: listen EACCES: permission denied ::1:62419
    at Server.setupListenHandle [as _listen2] (node:net:1881:21)
    at listenInCluster (node:net:1946:12)
    at GetAddrInfoReqWrap.doListen [as callback] (node:net:2116:7)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:111:8)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1925:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EACCES',
  errno: -4092,
  syscall: 'listen',
  address: '::1',
  port: 62419
}

I'll also include a solution in my PR to solve this issue. My proposal is to choose another port, like 62692, for Windows. This will also require you to add this URL to the whitelist in auth0.

I've played around with the code I can ensure a quick turnaround time for this project.

hoklims commented 5 days ago

/attempt #13