Experimental OE fixes #1859
Open
Experimental OE fixes #1859
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We currently have a React app deployed at `https://tps.autark.xyz/tps-auth` which does basically what this Authenticator does. Here's how it works: 1. Projects opens an popup which loads the GitHub OAuth login page 2. We have [a GitHub OAuth App][1], where we provide an Authorization Callback URL. This is configured to that `tps.autark.xyz` page. 3. This Authorizor receives a `code` from GitHub, which it posts back to Projects using `opener.postMessage` 4. Projects uses this `code` to hit `https://tps.autark.xyz/authenticate`, which holds onto the private key for our GitHub OAuth App and can thus load an API `token` for the user, so Projects can start making requests as this user. This commit seeks to optimize three aspects of the current approach: 1. Code visibility/maintainability: rather than having a deployed app with seemingly no corresponding GitHub repository, this adds this key piece of our infrastructure directly to our main `open-enterprise` repo. It also simplifies this Authenticator, using a simple html page rather than a complex React app. 2. Design: rather than a sans-serif "loading" message, this gives a simple visual while the user is logged in, with a subtle technical status 3. Security: loading this app at a URL we own provides no additional security. Why? Because any app, not just Projects, could open a GitHub OAuth page and provide our CLIENT_ID. GitHub would then redirect back to our Authenticator, regardless of where it lives, and Authenticator would blindly pass the `code` it received back to `opener`. From there, a malicious app can easily POST that code to our `https://tps.autark.xyz/authenticate` endpoint and receive a token for the user. And then that malicious app could read a user's private repos and post comments as them, and it would look like these GitHub actions were mediated by us. Unfortunately, the solution in this commit fails to address this security concern. It tries to read `opener.location.pathname` so that it can check the identity of the calling app, before passing `code` back to it, but calling `opener.location.pathname` is forbidden, because this Authenticator lives at a different domain than Projects. Or rather, Projects is served from the same IPFS gateway, BUT since it is rendered within an iframe, the browser treats it as living at a different URL than the page in the popup. IPFS hash of this version: QmSYYEAFJiPyPd6JGtreu8tQZwEjARDxGZpSnTEvex8K4C [1]: https://github.com/organizations/AutarkLabs/settings/applications/953918
Since Authenticator is not allowed to read `opener.location.pathname` directly, here's how this version works: 1. Projects opens a popup with the GitHub auth page 2. Our GitHub OAuth App is configured to redirect to the deployed Authenticator app (the index.html file here) 3. The Authenticator pings the `opener` (the Projects app) 4. Projects listens for this ping, and pings right back 5. Authenticator listens for pings from `opener`, and reads `origin` from these ping events. This DOES NOT WORK for Aragon apps. It would work for apps that are not loaded within an iframe, or for iframes that have [the `allow-same-origin` property set in their `sandbox` attribute][1] (changing this security setting is [not in the cards][2] for Aragon apps). But for regular iframes such as any Aragon app, when they send `postMessage` to the popup, the popup tries to read `message.origin`, and all it gets is `"null"`. IPFS hash for this version: QmaQFvQoGnWpsfLAumBJdSxV2zs9E6W5DPNB1fjg1QPxCu [1]: https://stackoverflow.com/questions/37838875/postmessage-from-a-sandboxed-iframe-to-the-main-window-origin-is-always-null [2]: https://github.com/aragon/aragon/blob/master/src/security/configuration.js#L22
This does not work, because the GitHub login page forbids being loaded within an iframe using a [`frame-ancestors 'none'` Content Security Policy][1] IPFS hash of this version: QmbC4CupvRo3JnducA8vdg9VmTMNCuUWs4eQiTACgLtxru [1]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors
This sends the `code` provided by GitHub to `window.opener`, no questions asked. IPFS hash of this version: QmNSdB6BpqbaYA6Vh38MBrUFJbrnjhsBRb8gq5o44ypoWu Note that I've also added `useCallback` here to prevent needless re-definition of `handleMessage` and associated removal & re-addition of the event listener.
The addition of the aragonUI Link caused all elements within the card to inherit the `white-space: nowrap` rule, breaking the line wrapping
* using `PropTypes.node` fixes irrelevant error when passing an array of elements (which is valid) * the `checked` property is *not* required
We need to work pretty hard to override some rules from the aragonUI Link component, which were mostly intended for Button
Allocations/Budget Card: wrap title to two lines
Markdown formatting fixes
Create Authenticator app; update Projects
implement commit linting for conventional-commit rules
fix: single scrollbar on New Project panel
While previously configuring lerna to run in parallel, it was introduced a regression that make test script fails because sharing devchain data, it was fixed by limiting concurrency to single thread
This change allows for easier E2E testing of the project app. While it is a less robust verification, the impact of a potential exploit (intercepting a read-only github token request) is negligible
- update s to ms - pass timeout value into an object so it is parsed correctly
- update tests to work with most recent app updates - create test-user token with a testrepo for use with projects
ghost
requested a review
from fix: removed fulfilled bounties from bounties screen
- fix user proptypes and incorporate decoupled user - remove milestone property - change issue data fall back from `undefined` to an empty object - remove default repo name and owner properties set for gql query - short-circuit repo gql query if the repo is decoupled - remove unnecessary decoupled check - add issueId to the update array for `useMemo`
fix(projects): acceptance of any submission
…ation Decentralized project integration
fix(projects): index issues on unique issue ID
…ation Decentralized project integration
fix: allocations panel help dialog uses correct period
- Added information on bash variables - Added permissions setup examples in each app installation section - Added recommendations around Projects app permissions - Updated sample commands to use bash variables
fix: small fixes for projects
- switch id over to issue ID from issue number - ensure repo ID stays consistent fro decoupled issues - ensure issue shape stays consistent through entire lifecycle
I noticed that the "Link" component inside the submission and application text wasn't displaying inline. I had to add an "href" prop to get it to render as a proper link element (as opposed to a button element) and that allowed me to get all submission text inline with the IdentityBadge and wrapping as expected
Decoupled Projects fixes
chore(dev-template): add explicit visibility
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates the react api for all the apps and changes the Discussion module to align with the Figma:
