Adding an option to disable admin panel

[agr]

Progress on https://github.com/NuGet/Engineering/issues/3418

Adds an AdminPanelEnabled option to the configuration (defaulted to true) that controls whether Admin area of the site gets registered as well as some admin routes not directly under /Admin.

AdminHelper was added for ease of checking and caching the results. AdminAction replaced the UIAuthorize(Roles = "Admins") to contain the check.

In order to also remove the capability of overriding permission checks by admin at some changes were done to the ActionsRequiringPermissions: a flag indicating whether admin is enabled or not is added and constants indicating sets of requirements were replaced with properties that return different values depending on that flag. Added tests that cover those differences.

[joelverhagen]

I mainly just have questions.

[loic-sharma]

Nice tests

[zhhyu]

I am a little curious about the risk without overriding these "Equals" and "GetHashCode" methods. I see we mainly use bit operations for these permissions. My apologies that I may miss something here.

[agr]

The default behavior of Equals and GetHashCode is to provide results based on objects address in memory (Equals only returns true if object is compared with itself and GetHashCode does some math on object's address and returns it).

Now, the change I made replaced consts in ActionsRequiringPermissions with properties (see DisplayPrivatePackageMetadata and below). So where previously if you have an ActionRequiringEntityPermissions<TEntity> instance and want to check if it is ActionsRequiringPermissions.DisplayPrivatePackageMetadata, you could just compare them as is, so as long as you stick to using those constants and not creating instances on your own, it would work (since you are dealing with the same objects all the time). With my change now every access to ActionsRequiringPermissions.DisplayPrivatePackageMetadata produces an new instance and simple equality comparison won't do anymore. In order for that to work I had to override the Equals implementation to compare properties of an object to establish equality. And if you override Equals you are pretty much required to override GetHashCode so it produces consistent results with Equals (so that if a.Equals(b) is true, a.GetHashCode() == b.GetHashCode() must also be true). Xoring integers and object hashes is a cheap way to achieve this.

Next, Moq's Verify (and Setup) method uses Equals when matches actual calls with templates in the code, so when test wants to verify (or setup) that a certain method was called passing certain value of permission (see this for an example:

NuGetGallery/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs

Lines 1762 to 1768 in cc653c1

	controller.MockApiScopeEvaluator
	.Verify(x => x.Evaluate(
	currentUser,
	It.IsAny<IEnumerable<Scope>>(),
	ActionsRequiringPermissions.UnlistOrRelistPackage,
	package.PackageRegistration,
	NuGetScopes.PackageUnlist));

), with properties instead of constants, the matching fails and test fails as a result. Those methods were introduced to address that test issue. Those methods are not used to actually evaluate permissions.

Questions answered, Joel is OOF