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

Regression: Client settings lost on save in UI #407

Closed
ben-jacobs opened this issue Apr 1, 2021 · 11 comments · Fixed by #409 or #420
Closed

Regression: Client settings lost on save in UI #407

ben-jacobs opened this issue Apr 1, 2021 · 11 comments · Fixed by #409 or #420
Labels
bug Something isn't working

Comments

@ben-jacobs
Copy link

Howdy :)

When using RavenDB as store, Clients created prior to 2bb302 lose settings on save if edited in the UI (no changes made to the affected properties).

Before:
{... "AllowedGrantTypes": [ { "Id": "clientgranttype/d90b4ce8-fd22-44d5-8603-fa65e9dd69cb", "ClientId": null, "GrantType": null, "CreatedAt": "0001-01-01T00:00:00.0000000", "ModifiedAt": null, "Client": null } ], "RedirectUris": [ { "Id": "clienturi/a142fea6-1b57-4718-83f4-36bc78b3bfe2", "ClientId": null, "Uri": null, "SanetizedCorsUri": null, "Kind": 0, "CreatedAt": "0001-01-01T00:00:00.0000000", "ModifiedAt": null, "Client": null } ], "AllowedScopes": [ { "Id": "clientscope/8b9ec9b8-54a9-4b82-b847-3110012c3e03", "ClientId": null, "Scope": null, "CreatedAt": "0001-01-01T00:00:00.0000000", "ModifiedAt": null, "Client": null } ], ... }

After:
"AllowedGrantTypes": null, "RedirectUris": null, "AllowedScopes": null,

@ben-jacobs ben-jacobs added the bug Something isn't working label Apr 1, 2021
@aguacongas
Copy link
Member

Hi Ben, Do you use the master branch ? Did you pull latest change ?

@aguacongas
Copy link
Member

Oups sorry you give me the commit :-(

@ben-jacobs
Copy link
Author

Yes master was where I noticed the problem (b9c191), but I worked backwards a bit and it also seemed to happen on 2bb302 -
Unfortunately I can't tell which commit I created the clients on, but its within the last couple of days since you merged the ravendb branch.

I'm wondering if it's related to that code cleanup you did to remove dead code? (haven't done any investigation yet)

@ben-jacobs
Copy link
Author

ben-jacobs commented Apr 1, 2021

If it helps, I just spun up master, initialised the clients etc. in a fresh DB, edited the client in the UI and all the settings were lost in the DB (Razor app crashed too). Almost certainly the app crash is because the API calls would have been rejected because of the lost settings on the client (i.e. the AllowedScopes)

@aguacongas
Copy link
Member

Probably, can you post the crach stack trace please

@aguacongas
Copy link
Member

@ben-jacobs can you try the fix on branch feature/ravendb ?

@ben-jacobs
Copy link
Author

ben-jacobs commented Apr 2, 2021

As requested the client stack trace when client dies following saving the changes to a client in the UI.

I think the client exception might be unrelated (not sure yet) ... I'm working on a repro, but in some cases the settings will be lost on save without the exception being thrown.
My culture by the way is en-AU and I'm getting plenty of console warning about missing resources.

blazor.webassembly.js:1 crit: Microsoft.AspNetCore.Components.WebAssembly.Rendering.WebAssemblyRenderer[100]
      Unhandled exception rendering component: Exception has been thrown by the target of an invocation.
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.ArgumentNullException: Value cannot be null. (Parameter 'source')
   at System.Linq.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at System.Linq.Enumerable.Where[ClientLocalizedResource](IEnumerable`1 source, Func`2 predicate)
   at Aguacongas.TheIdServer.BlazorApp.Validators.EntityResourceValidator`1[[Aguacongas.IdentityServer.Store.Entity.ClientLocalizedResource, Aguacongas.IdentityServer.Store, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]]..ctor(ILocalizable`1 model, EntityResourceKind kind, IStringLocalizer localizer)
   at Aguacongas.TheIdServer.BlazorApp.Validators.ClientValidator..ctor(Client client, IStringLocalizer localizer)
   at System.Reflection.RuntimeConstructorInfo.InternalInvoke(Object obj, Object[] parameters, Boolean wrapExceptions)
   --- End of inner exception stack trace ---
   at System.Reflection.RuntimeConstructorInfo.InternalInvoke(Object obj, Object[] parameters, Boolean wrapExceptions)
   at System.Reflection.RuntimeConstructorInfo.DoInvoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)
   at System.Activator.CreateInstance(Type type, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes)
   at System.Activator.CreateInstance(Type type, Object[] args)
   at Microsoft.AspNetCore.Components.Forms.EditContextExtensions.GetValidatorForModel(Object entity, Object model, IStringLocalizer localizer)
   at Microsoft.AspNetCore.Components.Forms.EditContextExtensions.ValidateModel(EditContext editContext, ValidationMessageStore messages, IStringLocalizer localizer)
   at Microsoft.AspNetCore.Components.Forms.EditContextExtensions.<>c__DisplayClass0_0.<AddFluentValidation>b__0(Object sender, ValidationRequestedEventArgs eventArgs)
   at Microsoft.AspNetCore.Components.Forms.EditContext.Validate()
   at Aguacongas.TheIdServer.BlazorApp.Pages.EntityModel`1[[Aguacongas.IdentityServer.Store.Entity.Client, Aguacongas.IdentityServer.Store, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].HandleModificationState_OnStateChange(ModificationKind kind, Object _)
   at Aguacongas.TheIdServer.BlazorApp.Services.HandleModificationState.EntityDeleted[ClientLocalizedResource](ClientLocalizedResource entity)
   at Aguacongas.TheIdServer.BlazorApp.Components.EntityResourcesComponentBase`1[[Aguacongas.IdentityServer.Store.Entity.ClientLocalizedResource, Aguacongas.IdentityServer.Store, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnDeleteResource(IEntityResource resource)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Components.ComponentBase.CallStateHasChangedOnAsyncCompletion(Task task)
   at Microsoft.AspNetCore.Components.ComponentBase.CallStateHasChangedOnAsyncCompletion(Task task)
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.GetErrorHandledTask(Task taskToHandle)

@ben-jacobs
Copy link
Author

So here's a bit of a repro -

Edit record, save and NOT lose settings.

  1. Edit an authorization_code client (as an example, happens with other types)
  2. Add an an additional scope
  3. Save the record

Edit record, save and settings lost:

  1. Edit an authorization_code client (as an example, happens with other types)
  2. Edit the description
  3. Tab out of the description field
  4. Save
    .... settings lost.

aguacongas pushed a commit that referenced this issue Apr 2, 2021
@aguacongas
Copy link
Member

Thanks, it should be fixed now, can you confirm ?

@ben-jacobs
Copy link
Author

Tested on a freshly initialized DB, repro as above does not produce error (settings saved).
Generally messed with changing settings for clients etc and couldn't see any errors, but didn't necessarily test all possible paths.

Do you think it's likely that this issue will affect other entities like the APIs/Providers/etc ?

@aguacongas
Copy link
Member

It was, because it was removing all navigation properties on update, but it's fix now .
Can I ask you to review the doc changed regarding RavenDb config on #408 ?

aguacongas pushed a commit that referenced this issue Apr 3, 2021
# [2.3.0](2.2.0...2.3.0) (2021-04-03)

### Bug Fixes

* add role store test ([caeb0a3](caeb0a3))
* cannot add localized resource ([fd5d9f5](fd5d9f5))
* cannot set client IDP restriction ([9835e82](9835e82))
* client settings lost on save in UI ([5b83178](5b83178)), closes [#407](#407)
* request on datetime ([ff15d71](ff15d71)), closes [#399](#399)
* set valid audience ([d1bd88a](d1bd88a)), closes [#388](#388)
* storage kind filesystem typo ([8350b39](8350b39)), closes [#397](#397)
* **ravendb:** update entity remove navigation props ([6a12d69](6a12d69)), closes [#407](#407)
* update packages ([721a47b](721a47b))
* update packages ([4f78aae](4f78aae))
* update packages ([b7f8c8c](b7f8c8c))
* update packages ([de160ce](de160ce))
* update packages ([aff1c9f](aff1c9f))
* update packages ([3d4df84](3d4df84))
* update packages ([cef74c4](cef74c4))
* update packages ([23254cc](23254cc))
* update packages ([86127c0](86127c0))
* update packages ([f94b3dc](f94b3dc))
* update packages ([47ab913](47ab913))

### Features

* ravendb ([c050afa](c050afa))
* wsfederation provider ([0295b1f](0295b1f)), closes [#403](#403)
aguacongas pushed a commit that referenced this issue Apr 7, 2021
# [2.3.0](2.2.0...2.3.0) (2021-04-07)

### Bug Fixes

* add role store test ([caeb0a3](caeb0a3))
* cannot add localized resource ([fd5d9f5](fd5d9f5))
* cannot set client IDP restriction ([9835e82](9835e82))
* client settings lost on save in UI ([5b83178](5b83178)), closes [#407](#407)
* request on datetime ([ff15d71](ff15d71)), closes [#399](#399)
* set valid audience ([d1bd88a](d1bd88a)), closes [#388](#388)
* storage kind filesystem typo ([8350b39](8350b39)), closes [#397](#397)
* **ravendb:** update entity remove navigation props ([6a12d69](6a12d69)), closes [#407](#407)
* update packages ([269e31d](269e31d))
* update packages ([d9f2cb0](d9f2cb0))
* update packages ([721a47b](721a47b))
* update packages ([4f78aae](4f78aae))
* update packages ([b7f8c8c](b7f8c8c))
* update packages ([de160ce](de160ce))
* update packages ([aff1c9f](aff1c9f))
* update packages ([3d4df84](3d4df84))
* update packages ([cef74c4](cef74c4))
* update packages ([23254cc](23254cc))
* update packages ([86127c0](86127c0))
* update packages ([f94b3dc](f94b3dc))
* update packages ([47ab913](47ab913))

### Features

* ravendb ([c050afa](c050afa))
* wsfederation provider ([0295b1f](0295b1f)), closes [#403](#403)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants