bookmarks_movement: restrict bookmarks that are marked allow_only_external_sync

Reviewed By: StanislavGlebik

Differential Revision: D23294907

fbshipit-source-id: ed89e5fd841e7d516b5d259c1f5de4e9f8f40ee3
This commit is contained in:
Mark Thomas 2020-08-25 09:10:07 -07:00 committed by Facebook GitHub Bot
parent eed7df1c52
commit f15976637e
13 changed files with 61 additions and 20 deletions

View File

@ -1,4 +1,4 @@
// @generated SignedSource<<a0441d92a2b0f30d635567caeb9bbc8c>>
// @generated SignedSource<<b696f1bd2a0295810f7667a98488b214>>
// DO NOT EDIT THIS FILE MANUALLY!
// This file is a mechanical copy of the version in the configerator repo. To
// modify it, edit the copy in the configerator repo instead and copy it over by
@ -291,9 +291,15 @@ struct RawBookmarkConfig {
3: list<RawBookmarkHook> hooks,
// Are non fastforward moves allowed for this bookmark
4: bool only_fast_forward,
// Only users matching this pattern (regex) will be allowed
// to move this bookmark
// If specified, and if the user's unixname is known, only users matching
// this pattern (regex) will be allowed to move this bookmark.
5: optional string allowed_users,
// If true, only external sync will be allowed to move this
// bookmark.
8: optional bool allow_only_external_sync,
// Whether or not to rewrite dates when processing pushrebase pushes
6: optional bool rewrite_dates,

View File

@ -105,13 +105,13 @@ impl<'op> CreateBookmarkOp<'op> {
bookmark_attrs: &'op BookmarkAttrs,
hook_manager: &'op HookManager,
) -> Result<(), BookmarkMovementError> {
self.auth
.check_authorized(ctx, bookmark_attrs, self.bookmark)?;
let kind = self
.kind_restrictions
.check_kind(infinitepush_params, self.bookmark)?;
self.auth
.check_authorized(ctx, bookmark_attrs, self.bookmark, kind)?;
self.affected_changesets
.check_restrictions(
ctx,

View File

@ -74,13 +74,13 @@ impl<'op> DeleteBookmarkOp<'op> {
infinitepush_params: &'op InfinitepushParams,
bookmark_attrs: &'op BookmarkAttrs,
) -> Result<(), BookmarkMovementError> {
self.auth
.check_authorized(ctx, bookmark_attrs, self.bookmark)?;
let kind = self
.kind_restrictions
.check_kind(infinitepush_params, self.bookmark)?;
self.auth
.check_authorized(ctx, bookmark_attrs, self.bookmark, kind)?;
if kind == BookmarkKind::Scratch || bookmark_attrs.is_fast_forward_only(self.bookmark) {
// Cannot delete scratch or fast-forward-only bookmarks.
return Err(BookmarkMovementError::DeletionProhibited {

View File

@ -95,8 +95,12 @@ impl<'op> PushrebaseOntoBookmarkOp<'op> {
bookmark_attrs: &'op BookmarkAttrs,
hook_manager: &'op HookManager,
) -> Result<pushrebase::PushrebaseOutcome, BookmarkMovementError> {
let kind = self
.kind_restrictions
.check_kind(infinitepush_params, self.bookmark)?;
self.auth
.check_authorized(ctx, bookmark_attrs, self.bookmark)?;
.check_authorized(ctx, bookmark_attrs, self.bookmark, kind)?;
if pushrebase_params.block_merges {
let any_merges = self
@ -114,10 +118,6 @@ impl<'op> PushrebaseOntoBookmarkOp<'op> {
}
}
let kind = self
.kind_restrictions
.check_kind(infinitepush_params, self.bookmark)?;
self.affected_changesets
.check_restrictions(
ctx,

View File

@ -5,6 +5,7 @@
* GNU General Public License version 2.
*/
use anyhow::anyhow;
use bookmarks_types::BookmarkName;
use context::CoreContext;
use metaconfig_types::{BookmarkAttrs, InfinitepushParams, SourceControlServiceParams};
@ -32,9 +33,20 @@ impl<'params> BookmarkMoveAuthorization<'params> {
ctx: &CoreContext,
bookmark_attrs: &BookmarkAttrs,
bookmark: &BookmarkName,
bookmark_kind: BookmarkKind,
) -> Result<(), BookmarkMovementError> {
match self {
BookmarkMoveAuthorization::User => {
if bookmark_kind == BookmarkKind::Public
&& bookmark_attrs.is_only_external_sync_allowed(bookmark)
&& !ctx.session().is_external_sync()
{
return Err(anyhow!(
"Only external sync is permitted to modify '{}'",
bookmark
)
.into());
}
if let Some(user) = ctx.user_unix_name() {
// TODO: clean up `is_allowed_user` to avoid this clone.
if !bookmark_attrs.is_allowed_user(&Some(user.clone()), bookmark) {

View File

@ -158,13 +158,13 @@ impl<'op> UpdateBookmarkOp<'op> {
bookmark_attrs: &'op BookmarkAttrs,
hook_manager: &'op HookManager,
) -> Result<(), BookmarkMovementError> {
self.auth
.check_authorized(ctx, bookmark_attrs, self.bookmark)?;
let kind = self
.kind_restrictions
.check_kind(infinitepush_params, self.bookmark)?;
self.auth
.check_authorized(ctx, bookmark_attrs, self.bookmark, kind)?;
self.update_policy
.check_update_permitted(
ctx,

View File

@ -1151,6 +1151,7 @@ fn test_verify_integrity_fast_failure(fb: FacebookInit) {
hooks: vec!["verify_integrity".into()],
only_fast_forward: false,
allowed_users: None,
allow_only_external_sync: None,
rewrite_dates: None,
hooks_skip_ancestors_of: vec![],
}];
@ -1177,6 +1178,7 @@ fn test_load_hooks_bad_rust_hook(fb: FacebookInit) {
hooks: vec!["hook1".into()],
only_fast_forward: false,
allowed_users: None,
allow_only_external_sync: None,
rewrite_dates: None,
hooks_skip_ancestors_of: vec![],
}];
@ -1229,6 +1231,7 @@ fn test_load_disabled_hooks_referenced_by_bookmark(fb: FacebookInit) {
hooks: vec!["hook1".into()],
only_fast_forward: false,
allowed_users: None,
allow_only_external_sync: None,
rewrite_dates: None,
hooks_skip_ancestors_of: vec![],
}];

View File

@ -1081,6 +1081,7 @@ mod test {
hooks: vec!["hook1".to_string(), "rust:rusthook".to_string()],
only_fast_forward: false,
allowed_users: Some(Regex::new("^(svcscm|twsvcscm)$").unwrap().into()),
allow_only_external_sync: None,
rewrite_dates: None,
hooks_skip_ancestors_of: vec![],
},
@ -1089,6 +1090,7 @@ mod test {
hooks: vec![],
only_fast_forward: false,
allowed_users: None,
allow_only_external_sync: None,
rewrite_dates: None,
hooks_skip_ancestors_of: vec![],
},

View File

@ -162,6 +162,7 @@ impl Convert for RawBookmarkConfig {
.map(|re| Regex::new(&re))
.transpose()?
.map(ComparableRegex::new);
let allow_only_external_sync = self.allow_only_external_sync;
let rewrite_dates = self.rewrite_dates;
let hooks_skip_ancestors_of = self
.hooks_skip_ancestors_of
@ -175,6 +176,7 @@ impl Convert for RawBookmarkConfig {
hooks,
only_fast_forward,
allowed_users,
allow_only_external_sync,
rewrite_dates,
hooks_skip_ancestors_of,
})

View File

@ -379,6 +379,12 @@ impl BookmarkAttrs {
}
}
}
/// Check if a bookmark is only updatable by an external sync process.
pub fn is_only_external_sync_allowed(&self, bookmark: &BookmarkName) -> bool {
self.select(bookmark)
.any(|params| params.allow_only_external_sync.unwrap_or(false))
}
}
/// Configuration for a bookmark
@ -394,6 +400,8 @@ pub struct BookmarkParams {
pub rewrite_dates: Option<bool>,
/// Only users matching this pattern will be allowed to move this bookmark
pub allowed_users: Option<ComparableRegex>,
/// Only the external sync job is allowed to modify this bookmark
pub allow_only_external_sync: Option<bool>,
/// Skip hooks for changesets that are already ancestors of these
/// bookmarks
pub hooks_skip_ancestors_of: Vec<BookmarkName>,

View File

@ -12,10 +12,10 @@ pub use session_id::SessionId;
pub use crate::core::CoreContext;
#[cfg(fbcode_build)]
pub use crate::facebook::is_quicksand;
pub use crate::facebook::{is_external_sync, is_quicksand};
pub use crate::logging::{LoggingContainer, SamplingKey};
#[cfg(not(fbcode_build))]
pub use crate::oss::is_quicksand;
pub use crate::oss::{is_external_sync, is_quicksand};
pub use crate::perf_counters::{PerfCounterType, PerfCounters};
pub use crate::session::{
generate_session_id, SessionClass, SessionContainer, SessionContainerBuilder,

View File

@ -15,6 +15,10 @@ pub fn is_quicksand(_ssh_env_vars: &SshEnvVars) -> bool {
false
}
pub fn is_external_sync(_ssh_env_vars: &SshEnvVars) -> bool {
false
}
impl CoreContext {
pub fn trace_upload(&self) -> impl Future<Item = (), Error = Error> {
ok(())

View File

@ -22,8 +22,8 @@ use tracing::TraceContext;
pub use self::builder::{generate_session_id, SessionContainerBuilder};
use crate::core::CoreContext;
use crate::is_quicksand;
use crate::logging::LoggingContainer;
use crate::{is_external_sync, is_quicksand};
mod builder;
@ -144,6 +144,10 @@ impl SessionContainer {
is_quicksand(self.ssh_env_vars())
}
pub fn is_external_sync(&self) -> bool {
is_external_sync(self.ssh_env_vars())
}
pub fn blobstore_read_limiter(&self) -> &Option<AsyncLimiter> {
&self.inner.blobstore_read_limiter
}