Fix an information disclosure when requesting access to a group containing private projects
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19102.
The commit speaks for itself:
Fix an information disclosure when requesting access to a group containing private projects
The issue was with the `User#groups` and `User#projects` associations
which goes through the `User#group_members` and `User#project_members`.
Initially I chose to use a secure approach by storing the requester's
user ID in `Member#created_by_id` instead of `Member#user_id` because I
was aware that there was a security risk since I didn't know the
codebase well enough.
Then during the review, we decided to change that and directly store the
requester's user ID into `Member#user_id` (for the sake of simplifying
the code I believe), meaning that every `group_members` / `project_members`
association would include the requesters by default...
My bad for not checking that all the `group_members` / `project_members`
associations and the ones that go through them (e.g. `Group#users` and
`Project#users`) were made safe with the `where(requested_at: nil)` /
`where(members: { requested_at: nil })` scopes.
Now they are all secure.
See merge request !1973
The issue was that `MergeRequest#mergeable?` returns false when the CI
state is not success and project.only_allow_merge_if_build_succeeds is
true. In this case `Projects::MergeRequestsController#merge` would
return the `:failed` status when enabling `merge_when_build_succeeds`,
thus leading to a weird state and the MR never automatically merged.
The fix is to disable the CI state check in the controller safeguard
that early return the `:failed` status.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Fix temp file being deleted after the request while importing a GitLab project
Fixes https://gitlab.com/gitlab-com/infrastructure/issues/151
In production, the temporary uploaded file is getting deleted straight after the request so the Sidekiq worker is unable to find it in `/tmp`
Also, improved erroring/logging of this situation.
See merge request !4894
The issue was with the `User#groups` and `User#projects` associations
which goes through the `User#group_members` and `User#project_members`.
Initially I chose to use a secure approach by storing the requester's
user ID in `Member#created_by_id` instead of `Member#user_id` because I
was aware that there was a security risk since I didn't know the
codebase well enough.
Then during the review, we decided to change that and directly store the
requester's user ID into `Member#user_id` (for the sake of simplifying
the code I believe), meaning that every `group_members` / `project_members`
association would include the requesters by default...
My bad for not checking that all the `group_members` / `project_members`
associations and the ones that go through them (e.g. `Group#users` and
`Project#users`) were made safe with the `where(requested_at: nil)` /
`where(members: { requested_at: nil })` scopes.
Now they are all secure.
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit changes the way certain documents are rendered (currently
only Notes) and how documents are redacted. Previously both rendering
and redacting would run on a per document basis. The result of this was
that for every document we'd have to run countless queries just to
figure out if we could display a set of links or not.
This commit changes things around so that redacting Markdown documents
is no longer tied into the html-pipeline Gem. This in turn allows it to
redact multiple documents in a single pass, thus reducing the number of
queries needed.
In turn rendering issue/merge request notes has been adjusted to take
advantage of this new setup. Instead of rendering Markdown somewhere
deep down in a view the Markdown is rendered and redacted in the
controller (taking the current user and all that into account). This has
been done in such a way that the "markdown()" helper method can still be
used on its own.
This particular commit also paves the way for caching rendered HTML on
object level. Right now there's an accessor method Note#note_html which
is used for setting/getting the rendered HTML. Once we cache HTML on row
level we can simply change this field to be a column and call a "save"
whenever needed and we're pretty much done.
Resolve "Destroying a project causes post_decline_request to be executed"
## What does this MR do?
Ensure we don't send "access request declined" to access requesters when a project is deleted.
## Are there points in the code the reviewer needs to double check?
I've created a service to decouple the notification sending from the AR model.
## Why was this MR needed?
Because there was an issue.
## What are the relevant issue numbers?
Fixes#18755, #18750.
## Does this MR meet the acceptance criteria?
- [x] No CHANGELOG needed.
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !4744
Make it possible to lock runner on a specific project
Make it possible to lock runner on a specific project.
----
----
Closes#3407
See merge request !4093
* upstream/master: (353 commits)
Put some admin settings in dropdown
Add styleguide on configuration settings documentation
Remove Duplicated keys add UNIQUE index to fingerprint
Avoid autoload issue such as 'Mail::Parsers::AddressStruct'
Move appearance settings as sub tab to application settings
use rails root join
fixed a couple of errors spotted in production
Fix RangeError exceptions when referring to issues or merge requests outside of max database values
Fix bug in `WikiLinkFilter`.
Grammar and typographic changes to artifacts documentation
Tweak grammar
Small frontend code fixes and restore 8a2d88f commit
Warn about admin privilege to disable GitHub Webhooks
Listing GH Webhooks doesn't stop import process for non GH admin users
fixup! updated docs for api endpoint award emoji
Update CHANGELOG
Ensure Todos counters doesn't count Todos for projects pending delete
Add endpoints for award emoji on notes
Sort API endpoints and implement feedback
Add endpoints for Award Emoji
...
This is to ensure we don't send unwanted notifications when deleting a
project. In other words, stop abusing AR callbacks and use services.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Implement custom notification level options
part of #12758
See merge request !4389
Ensure Todos counters doesn't count Todos for projects pending delete
Use `TodosFinder` instead of `current_user.todos` to filter projects
pending delete on Todos counters helpers.
Counters should not reflect the number of Todos displayed on the tabs.
Fixes#18633
See merge request !4663
Disable the unlink feature for SAML connected accounts (social login).
This disables the ability to manually unlink your SAML account, if you have one connected. In certain scenarios, the only allowed login mechanism can be SAML, and if you unlink your account you will be locked out of the system (configuration dependent).
Fixes#18613
See merge request !4662
- As todos are created/updated inside the TodoService
we repopulate the cache just there for both pending/done todos
- Todos as mark as done from the TodosController we update cache
there too
- All the added methods are kept in the User class for cohesion
Add sorting dropdown to tag page
## What does this MR do?
Adds a sorting dropdown to the tags page just like the one on the branches page.
## Are there points in the code the reviewer needs to double check?
No
## Why was this MR needed?
Clients were asking for this
## What are the relevant issue numbers?
Closes#14918
## Screenshots (if relevant)
See merge request !4423
Allow creating Personal Access Tokens through the website
Related to #2979
- Allow a user to create personal access tokens, and use them to authenticate
- Refactor `API::Helpers` into `API::Helpers::Core` and `API::Helpers::Authentication`
# Tasks
- [ ] #2979 (!3749) - Personal Access Tokens
- [x] Basic Implementation
- [x] Add UI to add "Personal Access Tokens"
- [x] Reload `lib/api` on every request
- [x] Respect these tokens for API requests
- [x] Just a param or a header too?
- [x] Allow revoking tokens
- [x] Expire tokens
- [x] Left bar should have a "PAT" icon
- [x] Scopes?
- [x] Copy to Clipboard
- [x] Show active/inactive tokens separately
- [x] No need to check for expired/revoked in the appropriate places
- [x] Why does regular ApplicationController check for private token?
- [x] Support non-API requests
- [x] Revert (or work on) `lib/api` eager loading
- [x] Create MR
- [x] Refactoring
- [x] Fix tests
- [x] Write more tests
- [x] Add screenshots to MR
- [x] Add description of query performance to MR
- [x] Limit the number of queries in the `personal_access_tokens` page
- [x] Wait for CI to pass
- [x] Fix merge issues in schema.rb
- [x] Assign MR to endboss
- [x] Wait for feedback
- [x] Fix feedback
- [x] Wait for CI to pass
- [x] Assign to @rspeicher
- [x] Fix @rspeicher's comments
- [x] Wait for CI to pass
- [x] Assign back to @rspeicher
- [x] Write documentation and ping @axil
- [x] Wait for Axil to respond
- [x] Assign to endboss
- [x] Address Douwe's feedback
- [x] Use the `private_token` or `authentication_token` param instead of `personal_access_token`
- [x] Ditto for the header
- [x] Assign to endboss
- [x] Make sure CI is green
- [x] Address Douwe's feedback
- [x] Don't go through the `authenticate_user_from_private_token!` method, if a private token is supplied (or combine them)
- [x] In `authenticate_user_from_personal_access_token!` don't hit DB if `token_string` is `nil`
- [x] Use `current_user.personal_access_tokens.build` in the controller
- [x] Remove the "We aren't using `personal_access_token` as the root param" comment
- [x] `No need for = "...", we can just have the Inactive ... #{...} on the next line` in the view
- [x] Render dates in a (more) human format
- [x] CSS issue with table
- [x] Don't show the tokens in the UI indefinitely
- [x] How to implement scopes? Add-on to current impl? Doorkeeper?
- [x] Wait for @DouweM's comments about scopes
- [x] Address @DouweM's second review
- [x] Try not using `native['innerHTML']`
- [x] use contexts for all "when ..."
- [x] Ensure consistency (styling) with other pages for "You don't have any tokens" message
- [x] "Actions" table column doesn't need a label
- [x] %td can be moved outside of the if/else statement
- [x] The header title should be "Profile Settings"
- [x] Can this be a `before_create`, so we don't need to use `generate`?
- [x] If it couldn't be revoked, will we show an error?
- [x] If it couldn't be saved, will we show an error?
- [x] Merge master
- [x] Update CHANGELOG entry
- [x] Add tests for form errors?
- [x] Post screenshots
- [x] Tag @jschatz1 for review
- [x] Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/0dff6fd/builds) to pass
- [x] Respond to @jschatz1's comments
- [x] Hardcoded colors should be variables
- [x] Should not be allowed to chose a date in the past
- [x] Use the same table as in the Applications tab
- [x] button should say "Create Personal Access Token"
- [x] Float the revoke to the right on the `a`
- [x] Change revocation message. "Are you sure you want to revoke this certificate? This action cannot be undone."
- [x] Date stays selected and looks selected even though date is set as "never".
- [x] ~~hover on the calendar button shifts~~ (not caused by this MR - happens on `milestones#new` as well)
- [x] Don't use the panel for the created token
- [x] Use a normal flash for "Your new personal access token has been created"
- [x] Show the input (with the token) below it full width.
- [x] Put the "Make sure you save it - you won't be able to access it again." message near the input
- [x] Have the input highlight all on single click
- [x] Update screenshots
- [x] Merge master in + conflicts
- [x] Assign to @jschatz1 again
- [x] Respond to @jschatz1's comments
- [x] No button for clipboard, only link
- [x] text-danger
- [x] highlight fade on that area where the token was created
- [x] Make sure [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/d754d99179f1ffe846fcc1d8e858163b39efc5dc/builds) is green
- [x] Assign to @jschatz1
- [x] Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/faa0e3f7580bc38d4d12916b4589c64d6c2678a7/builds) to pass
- [x] Respond to @DouweM's feedback
- [x] move the redirect_to out of the if/else
- [x] certificate -> token
- [x] datepicker back to text field
- [x] combine the get_user_from_private_token and get_user_from_personal_access_token methods in ApplicationController
- [x] combine the get_user_from_private_token and get_user_from_personal_access_token methods in `lib/api/helpers`
- [x] don't need the new constants
- [x] Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/9d7cda3ddce52baad9618466a5d00319b333be57/builds) to pass
- [ ] Wait for merge
# Screenshots
See merge request !3749
* upstream/master: (337 commits)
Update CHANGELOG for !4659
Center the header logo for all Devise emails
Add previews for all customized Devise emails
Customize the Devise `unlock_instructions` email
Customize the Devise `reset_password_instructions` email
Customize the Devise `password_change` emails
Use gitlab-git 10.2.0
Use Git cached counters on project show page
Fix indentation scss-lint errors
Added title attribute to enties in tree view Closes#18353
Banzai::Filter::ExternalLinkFilter use XPath
Reduce queries in IssueReferenceFilter
Use gitlab_git 10.1.4
Fixed ordering in Project.find_with_namespace
Fix images in emails
Banzai::Filter::UploadLinkFilter use XPath
Turn Group#owners into a has_many association
Make project_id nullable
CHANGELOG [ci skip]
CHANGELOG [ci skip]
...
Josh Frye
Robert Speicher
Robert Speicher
Douwe MaanandDouwe Maan
Stan Hu
Ruben Davila
Felipe Artur
Rémy Coutable
Yorick Peterse
James Lopez
Paco Guzman
Z.J. van de Weg
Jacob Schatz
Grzegorz Bizon
Lin Jen-Shin
Phil Hughes
Dmitriy Zaporozhets
Douglas Barbosa Alexandre