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

Add owner scope to API keys #4935

Merged
merged 2 commits into from
Nov 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/NuGetGallery.Core/Entities/EntitiesContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ protected override void OnModelCreating(DbModelBuilder modelBuilder)
modelBuilder.Entity<Scope>()
.HasKey(c => c.Key);

modelBuilder.Entity<Scope>()
.HasOptional(sc => sc.Owner)
.WithMany()
.HasForeignKey(sc => sc.OwnerKey)
.WillCascadeOnDelete(false);

modelBuilder.Entity<Scope>()
.HasRequired<Credential>(sc => sc.Credential)
.WithMany(cr => cr.Scopes)
Expand Down
23 changes: 22 additions & 1 deletion src/NuGetGallery.Core/Entities/Scope.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using Newtonsoft.Json;

Expand All @@ -16,6 +15,20 @@ public class Scope
[JsonIgnore]
public int CredentialKey { get; set; }

/// <summary>
/// Package owner (user or organization) scoping.
/// </summary>
[JsonProperty("o")]
public int? OwnerKey { get; set; }

/// <summary>
/// Package owner (user or organization) scoping.
/// </summary>
public User Owner { get; set; }

/// <summary>
/// Packages glob pattern.
/// </summary>
[JsonProperty("s")]
public string Subject { get; set; }

Expand All @@ -30,6 +43,14 @@ public Scope()
{
}

public Scope(User owner, string subject, string allowedAction)
{
Owner = owner;
Subject = subject;
AllowedAction = allowedAction;
}

// Deprecated: Should be removed once ApiKeys.cshtml is updated to support owner scope.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this just be as simple as changing a constructor call to pass the current username as owner?

You can add the ability to choose what user later. I would just prefer not having deprecated methods if it's very easy to fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but then I'd need to update all callers. I'm minimizing changes in this PR (schema and copying org scope for temp keys). I'll remove this ctor when I start requiring the owner, which happens when I enable the UI.

public Scope(string subject, string allowedAction)
{
Subject = subject;
Expand Down
2 changes: 1 addition & 1 deletion src/NuGetGallery/Controllers/ApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,9 @@ public async virtual Task<ActionResult> CreatePackageVerificationKeyAsync(string
// symbols and the VerifyPackageKey callback returns the appropriate response. For this reason, we
// always create a temp key scoped to the unverified package ID here and defer package and owner
// validation until the VerifyPackageKey call.
var user = GetCurrentUser();
var credential = CredentialBuilder.CreatePackageVerificationApiKey(id);

var user = GetCurrentUser();
await AuthenticationService.AddCredential(user, credential);

TelemetryService.TrackCreatePackageVerificationKeyEvent(id, version, user, User.Identity);
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions src/NuGetGallery/Migrations/201711021733062_ApiKeyOwnerScope.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
namespace NuGetGallery.Migrations
{
using System;
using System.Data.Entity.Migrations;

public partial class ApiKeyOwnerScope : DbMigration
{
public override void Up()
{
AddColumn("dbo.Scopes", "OwnerKey", c => c.Int());
CreateIndex("dbo.Scopes", "OwnerKey");
AddForeignKey("dbo.Scopes", "OwnerKey", "dbo.Users", "Key");
}

public override void Down()
{
DropForeignKey("dbo.Scopes", "OwnerKey", "dbo.Users");
DropIndex("dbo.Scopes", new[] { "OwnerKey" });
DropColumn("dbo.Scopes", "OwnerKey");
}
}
}
126 changes: 126 additions & 0 deletions src/NuGetGallery/Migrations/201711021733062_ApiKeyOwnerScope.resx

Large diffs are not rendered by default.

9 changes: 8 additions & 1 deletion src/NuGetGallery/NuGetGallery.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,10 @@
<Compile Include="Migrations\201710301857232_Organizations.Designer.cs">
<DependentUpon>201710301857232_Organizations.cs</DependentUpon>
</Compile>
<Compile Include="Migrations\201711021733062_ApiKeyOwnerScope.cs" />
<Compile Include="Migrations\201711021733062_ApiKeyOwnerScope.Designer.cs">
<DependentUpon>201711021733062_ApiKeyOwnerScope.cs</DependentUpon>
</Compile>
<Compile Include="Services\AsynchronousPackageValidationInitiator.cs" />
<Compile Include="Services\CloudBlobFileStorageService.cs" />
<Compile Include="Services\IFileStorageService.cs" />
Expand Down Expand Up @@ -1866,6 +1870,9 @@
<EmbeddedResource Include="Migrations\201710301857232_Organizations.resx">
<DependentUpon>201710301857232_Organizations.cs</DependentUpon>
</EmbeddedResource>
<EmbeddedResource Include="Migrations\201711021733062_ApiKeyOwnerScope.resx">
<DependentUpon>201711021733062_ApiKeyOwnerScope.cs</DependentUpon>
</EmbeddedResource>
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv1packages.json" />
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv1search.json" />
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv2getupdates.json" />
Expand Down Expand Up @@ -2296,4 +2303,4 @@
copy "$(ProjectDir)..\Bootstrap\dist\js\bootstrap.js" "$(ProjectDir)Scripts\gallery" &gt;NUL
</PreBuildEvent>
</PropertyGroup>
</Project>
</Project>