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

[Feature Request] make redirect_uri work until gitlab fix their issue. #99

Closed
mohan43u opened this issue Jul 2, 2020 · 7 comments
Closed
Labels
enhancement Enhancement request gitlab Issues about GitLab stale Stale issues or pull requests

Comments

@mohan43u
Copy link

mohan43u commented Jul 2, 2020

What problem does the feature solve?

gitlab oauth should work from any static page. Not just from the document root.

Problem: Currently gitlab oauth workflow is not working when user from a static website try to comment on a particular page other than the root page. This issue was already reported here. But it was closed citing this gitlab issue

Proposed solution

The default redirect_uri= value sent by Vssue to GitLab is window.location.href without [prefix], Instead of sending window.location.href, why not send redirect_uri=window.locaiton.origin with state property containing base64 encoded object with original state and local redirect_uri where this internal redirect_uri should contain window.location.href, so when oath completes, Vssue can check for redirect_uri in state and again redirect to the original url which initiated the oauth request.

@cybermoloch
Copy link

This sounds like a good fix since the OAuth standard discourages wildcard matching for the redirect URI and many other OAuth providers do the same as GitLab.

I did try this however and it does seem to break some parts of SPAs. Specifically I was attempting to use Vssue with Vuelog and the URL encoding breaks a few things. The encoding of the URL replacing all special characters gives 404 errors with Vuelog --this is especially an issue as it rewrites the URL bar in the browser which breaks a simple refresh of the page. Many SPAs use an anchor tag (#) in their URL for routing so even the current release of Vssue won't work as you cannot put # in the redirect URI either.

@cybermoloch
Copy link

As @meteorlxy mentioned in another issue (#76), this needs to be applied to all APIs. Passing the current URL as a Base64 string could work for all of them instead of passing the current URL encoded.

I think because we are passing the state into Base64, we may not need to encode it first. (Base64 will accept any single byte characters and any non ASCII characters should already be encoded by the browser as it would be an invalid URL otherwise?) As I previously mentioned, I am using this in an SPA with vue-router in hash mode (Vuelog). Although having vue-router work in history mode might help somewhat, many static hosting will not allow changing parameters for doing a catch-all redirect. (See https://router.vuejs.org/guide/essentials/history-mode.html#example-server-configurations for more info.) Even so, we need to make sure Vssue works in both modes.

Another consideration is that the window.location.origin without the full state loaded may not include the Vssue component so if Vssue is responsible for handling the state and going back to the original page, it may not work as intended. In that case, a new route for should be added and used as the routing for Vssue OAuth requests. An alternative would be to have a Vssue stub component to handle this on another page even if it doesn't allow comments but that seems messier.

@meteorlxy
Copy link
Owner

Sorry for delay.

I'm planning for Vssue 2.0 with Vue 3.0, which is expected to solve some known issues in a more elegant way.

I'll refer to that PR and our discussions here. Great thanks 😄

@mohan43u
Copy link
Author

@cybermoloch

I think because we are passing the state into Base64, we may not need to encode it first.

This patch not doing any URL encoding. We json stringify one state object then convert it into base64

https://github.com/meteorlxy/vssue/pull/101/files#diff-da508bd9be97a1016faa11587b990b23

No URL encoding involved here. Also, I'm so new to web and not able to understand most of the problems you explained in your comments. I use Vssue with this patch in my personal blog which works as intended. If possible, could you please provide some way to see how this patch not works in SPA?

@cybermoloch
Copy link

@mohan43u
The encoding I think is BEFORE you start to handle it with your modifications but it is a rabbit hole of functions that do it.

You can do this as a test on your local machine using npm and Vuelog. I have a fork here where I am adding support to Vuelog for Vssue. https://github.com/cybermoloch/Vuelog/tree/feature-vssue

  1. Download the code (git or zip) and run npm install
  2. Run npm run serve
  3. Modify database.js to enable Vssue (in public/userdata) and change the API to GitLab.
  4. Go to a blog post such as showcase -> style examples; the URL is http://localhost:8080/#/blog/showcase/2016/style-examples/ [Normal Vssue]
  5. Replace Vssue with your version and build it. (Download code, run yarn from directory, copy lib files for GitLab API to node_modules@vssue\api-gitlab-v4\lib in Vuelog directory.)
  6. Page will reload but will quickly replace the URL with http://localhost:8080/#%2Fblog%2Fshowcase%2F2016%2Fstyle-examples%2F=.
  7. Using this URL (refresh or otherwise) will result in the equivalent of a 404 error (Oops! The page you were looking for doesn't exist.) The actual code that does this URL replacement in the address bar is line 79 in the built js file: window.history.replaceState(null, '', replaceURL); but I am assuming it uses this as a base to form the rest of the base 64 operation, state handling operations.

@meteorlxy meteorlxy added enhancement Enhancement request gitlab Issues about GitLab labels Jul 24, 2020
@stale
Copy link

stale bot commented Sep 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale issues or pull requests label Sep 22, 2020
@stale stale bot closed this as completed Sep 30, 2020
@mohan43u
Copy link
Author

mohan43u commented Nov 15, 2020

@cybermoloch I did some fix to my patch to make sure hash string is not omitted after redirection. Kindly check if it works for you now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement request gitlab Issues about GitLab stale Stale issues or pull requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants