Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

Consider create datetime on ClientSecret #2055

Closed
LindaLawton opened this issue Feb 7, 2018 · 23 comments
Closed

Consider create datetime on ClientSecret #2055

LindaLawton opened this issue Feb 7, 2018 · 23 comments
Assignees
Milestone

Comments

@LindaLawton
Copy link
Contributor

Client secrets are created and we can have more then one. I was thinking that our developers may create several client secrets. It would be nice to know when a secret was created. My current solution is to add a time stamp to the description property which works but isn't ideal.

I am willing to submit a pull request on this if you agree its wroth adding.

@LindaLawton LindaLawton changed the title Consider create date on ClientSecret Consider create datetime on ClientSecret Feb 7, 2018
@leastprivilege
Copy link
Member

This would mean a schema change - and we try to avoid that..

I will put it in backlog.

@LindaLawton
Copy link
Contributor Author

LindaLawton commented Feb 7, 2018

Do you have any idea how much hair pulling i have had from the schema changes in 1.x -> 2.0 upgrade?

I kind of thought you guys loved schema changes :) Can we get migration steps in this time?

@brockallen
Copy link
Member

brockallen commented Feb 7, 2018

Can we get migration steps in this time

Migrations are the concern of the hosting application. This is because fundamentally EF migrations don't support library style akpplications.

@brockallen brockallen added this to the 3.0 milestone Feb 27, 2018
@brockallen
Copy link
Member

We'll add this on the 3.0 milestone, since that will be an expected big (schema) change.

@leastprivilege also is mentioning a "last access" time so people know which secrets are stale.

@brockallen brockallen self-assigned this Feb 27, 2018
@LindaLawton
Copy link
Contributor Author

LindaLawton commented Apr 4, 2018

Can we add a create datetime and a last access datetime for Clients as well. I am going to see about adding it to client properties for now.

@brockallen brockallen modified the milestones: 3.0, 2.3 Jun 4, 2018
@brockallen
Copy link
Member

Also, perhaps a LastAccessedDate

@LindaLawton
Copy link
Contributor Author

I keep meaning to try to add this as a clientproperty to the client validate. I just couldn't decide if lastaccessdate counted with failed login or just with successful logins.

we apparently need this for GDPR compliance.

@TomCJones
Copy link

TomCJones commented Jun 4, 2018

I am planning to add this to the user db to record last successful login as I use that to establish termination date, which is what I need for consent receipt for GDPR. BTW, I implemented consent receipt on a ASP.NET identity / idsvr4 OP. But I have not found any good way to have user consume the data as yet,

failed logons could be some sort of attack and not the user's access attempt

@LindaLawton
Copy link
Contributor Author

But in the event of client credentials Grant type its not a user and you would need to store lastaccessdate of the client itself.

what about refresh token usage do you think it best to store that on the user or the client or the client access grant can't remember that table I am mobile right now.

@TomCJones
Copy link

GDPR should only apply to user? or am I missing something here?
If you are talking about impersonation, I just refuse to allow that as it is far beyond GDPR thinking.
I don't think that there is anything that requires tracking refresh usage, altho I never thought much about it.

@LindaLawton
Copy link
Contributor Author

It is possible that this is a requirement from a customer I am not privileged to that information.

I was told anyone or anything accessing data must be logged. In the event of a client its the developer who we must log who's data they accessing and when oauth2 clients. If its an identity login then we log it as the user.

@brockallen
Copy link
Member

or am I missing something here

The values we're discussing here are client, not user related.

@LindaLawton
Copy link
Contributor Author

@brockallen Sorry we apart to have side tracked your thread.

I would be interested to hear how you plan on detecting last access? Are we talking last valid use of it or last attempted use of it?

Also probably another issue but a method for resetting secret would be really nice.

@brockallen
Copy link
Member

I would be interested to hear how you plan on detecting last access? Are we talking last valid use of it or last attempted use of it?

Yea, we'd have to find out in the authN code which one is matched and then somehow trigger an update to the DB for it.

@TomCJones
Copy link

TomCJones commented Jun 5, 2018 via email

@LindaLawton
Copy link
Contributor Author

Ok I am going to play devils advocate here and ask will updating the database on all successful logins cause any load issues? deadlocks can't happen on update iir but what happens if the row no longer exists for some reason or you can't access the database. there will need to be some readable error handling with this.

@TomCJones
Copy link

This thread does seem to be wandering, but you should consider the purpose of the field. If the record doesn't exist there is no purpose served in updating it.

1 similar comment
@TomCJones
Copy link

This thread does seem to be wandering, but you should consider the purpose of the field. If the record doesn't exist there is no purpose served in updating it.

@leastprivilege
Copy link
Member

I don't see this for 2.3 - will be considered at some later point.

@leastprivilege leastprivilege modified the milestones: 2.3, Future Aug 1, 2018
@leastprivilege
Copy link
Member

I misspoke ;)

@leastprivilege leastprivilege modified the milestones: Future, 2.3 Aug 1, 2018
@brockallen
Copy link
Member

68747470733a2f2f7062732e7477696d672e636f6d2f6d656469612f4469634e6a694255454141347456512e6a7067

@brockallen
Copy link
Member

Created date time added to the secret EF class.

@lock
Copy link

lock bot commented Jan 13, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants