Allow Visual Review Tool to post on behalf of logged in user
Problem to solve
In order to allow the Visual Review Tool to work with projects that are Internal
or Private
we need the user to paste their Personal Access Token directly into the VRT. This is likely cumbersome and error prone for Parker (Product Manager) and Presley (Product Designer) who would be the main users of this tool.
Intended users
Further details
Another issue is that the VRT currently posts comments as a Bot, and not as the actual user who is logged into the GitLab instance.
Proposal
Option 1: Shared Cookies
If we can use the GitLab instance session cookie as described in this thread then the Visual Review Tool could post comments authorized as the logged in user. This would allow users to interact with the Visual Review Tool for projects that are Internal or Private without pasting their Personal Access Token into it.
Option 2: OAuth
As described here we could implement an OAuth scheme that would allow the Visual Review Tool to authenticate itself through the GitLab instance and allow it to operate as the user.
There is some question here as to how we would operate an OAuth flow given that the review app domains are dynamic. We would have to do some research/investigation to figure this out
Option 3:
As described from here down we could embed some or most of the Visual Review Tool inside of an iframe. If we did this, then the iframe content could re-use the session information (viability TBD by AppSec) and could post on behalf of the user. This may also have the add-on benefit of removing issues with style collisions. We would still need a javascript component of the Visual Review Tool widget in order to capture the screenshot. After that javascript captured the screenshot it could use the browser postMessage API to pass the screenshot into the iframe, at which point the iframe'd page could upload it on behalf of the user to the GitLab instance.
Option 4: Pop-up window
As described here we could use a pop-up window to load a page on the GitLab instance in order to facilitate the posting of comments on behalf of a user.
Permissions and Security
Option 1: Shared Cookies
Since Rack-Cors and browsers (at least Chrome and Firefox...) will block CORS requests that allow *
and have Access-Control-Allow-Credentials: true
, we would need to be specific about which domains are eligible for using credentials on the visual review tool resources.
Because there could be many different domains used by review apps, and the main rails application would have to know a valid domain compared to a not valid one based on the specific project. This should be possible with rack-cors's dynamic origins feature. The source code outlines how it is invoked.
allow do
origins {|source, env| true || false} # maybe host_determination_proc or something similar
resource '/api/v4/projects/*/merge_requests/*/visual_review_discussions',
credentials: true,
headers: :any,
methods: :any,
expose: headers_to_expose
end
If we select this approach, after we overcome the dynamic origin hurdle there will likely be issues with overcoming CSRF protection. We would have to investigate how to address this if this approach is chosen.
The Visual Review tool is a javascript file that is hosted on GitLab instances and can be pulled in by review apps on a separate domain. Because the file is hosted on GitLab instances, requesting that file from a browser will include the users session cookie from the GitLab instance in question. We want to re-use that cookie from the review app domain in order to authenticate against a GitLab instance.
sequenceDiagram
GitLab-->>+ReviewApp: Deploy review app to foo.bar
Browser->>GitLab: Click review button in MR
GitLab->>Browser: Redirect to review app at foo.bar
Browser->>ReviewApp: GET foo.bar
Browser->>GitLab: GET GitLab.com/vrt.js
rect rgb(93 126 173)
Browser->>GitLab: POST <my comment> from vrt.js
end
The highlighted request in the above diagram is the one we wish to use the GitLab instance session cookie in, if possible. At that time the users browser is on foo.bar and not on the GitLab instance website.
If we can't re-use the session cookie, we would need AppSec to weigh in and let us know what the preferred authentication mechanism would be, without user interaction if possible.
Documentation
We would need to change the documentation to reflect the new functionality.
Availability & Testing
We are not sure if adding a proc inside of an allow do block in application.rb would cause it to be invoked at every request or only on relevant requests. If it is invoked on every request that would be inefficient.