DuendeSoftware / IdentityServer

The most flexible and standards-compliant OpenID Connect and OAuth 2.x framework for ASP.NET Core
https://duendesoftware.com/products/identityserver
Other
1.49k stars 349 forks source link

Client Secret usage audit #1614

Open jasonliao-cb opened 1 month ago

jasonliao-cb commented 1 month ago

Which version of Duende IdentityServer are you using? 6.1.8

Which version of .NET are you using? 6

Describe the bug

we have set the client secrets management so each client can have the secret rotated, but it is difficult to tell the last time a secret was used or the client's last activity. we see the AWS secret can have the last used information, is it possible we can save this info on the secret level so we can track the usage?

Expected behavior

A clear and concise description of what you expected to happen.

maybe a new field in the client secret table for the last used

jasonliao-cb commented 1 month ago

we also found it is not able to add CreatedBy and UpdatedBy fields to the ClientSecret table.

    public class OneIAMClientSecret : ClientSecret
    {
        public string CreatedBy { get; set; }
        public string UpdatedBy { get; set; }
    }

    public class ConfigurationDbContext : Duende.IdentityServer.EntityFramework.DbContexts.ConfigurationDbContext<ConfigurationDbContext>
    {
        public DbSet<OneIAMClientSecret> ClientSecrets { get; set; }
    }

while adding the migration it will throw an exception `Cannot use table 'clientsecrets' for entity type 'OneIAMClientSecret' since it is being used for entity type 'ClientSecret' and potentially other entity types, but there is no linking relationship. Add a foreign key to 'OneIAMClientSecret' on the primary key properties and pointing to the primary key on another entity type mapped to 'clientsecrets'.

if we map the OneIAMClientSecret to the same clientsecrets table

    public class ConfigurationDbContext : Duende.IdentityServer.EntityFramework.DbContexts.ConfigurationDbContext<ConfigurationDbContext>
    {
        public DbSet<ClientSecret> ClientSecrets { get; set; }

        public DbSet<OneIAMClientSecret> OneIAMClientSecrets { get; set; }

protected override void OnModelCreating(ModelBuilder builder)
        {
            base.OnModelCreating(builder);

            builder.Entity<OneIAMClientSecret>()
                .ToTable("clientsecrets");

            builder.Entity<OneIAMClientSecret>()
                .Property(o => o.CreatedBy)
                .HasMaxLength(100);

            builder.Entity<OneIAMClientSecret>()
                .Property(o => o.UpdatedBy)
                .HasMaxLength(100);
        }
    }

the migration does nothing. it's impossible for us to have the audit log now.

please help.

RolandGuijt commented 1 month ago

This does seem more related to Entity Framework than to our products. I would suggest trying my tip below and if that doesn't work please create an issue on the Entity Framework issue tracker.

The setup you're showing doesn't work because the ClientSecrets DbSet already exists in the parent DbContext ConfigurationDbContext. To add the columns, I recommend to create a separate entity that doesn't derive from ClientSecret with the needed columns and then create a one to one relationship between them in OnModelCreating. Example here.

jasonliao-cb commented 1 month ago

Hi @RolandGuijt that is not a good approach, as it will cause the new entity not to share the same information with the ClientSecret. The purpose is to have the client's secret usage fully audited, is it possible to design the identity table structure as the asp.net identity so we can extend and implement some custom fields?

ApplicationDbContext : IdentityDbContext<ApplicationUser>
public class ApplicationUser : IdentityUser
{
        public string CompanyName { get; set; }
}

I think this would be a great feature if you can provide it to business/enterprise customers

AndersAbel commented 1 month ago

@jasonliao-cb Adding usage auditing to the secrets is a good idea and it looks like our existing API makes it hard to add that as an extension.

I'm transferring this issue to the IdentityServer repo to be considered for the development roadmap.

I think that this issue brings up two different features:

  1. The specific client secret auditing
  2. The ability to extend the configuration entries in the database with custom properties.

If we implement this I think we should add client secret auditing as a built in behaviour, but also consider general extensibility.

jasonliao-cb commented 1 month ago

Thanks, @AndersAbel, for moving this here.

Is there any expectation on which version/schedule we can expect to see this feature implemented? Our company uses Identity Server as our base framework and we've extended some features. The security audit feature is a higher priority for us, and we are keen to know its timeline.