feat: add oauth2 codes and tokens to database #11779
Conversation
Might have no match, and a -1 will panic.
| q.oauth2ProviderAppCodes[index] = q.oauth2ProviderAppCodes[len(q.oauth2ProviderAppCodes)-1] | ||
| q.oauth2ProviderAppCodes = q.oauth2ProviderAppCodes[:len(q.oauth2ProviderAppCodes)-1] |
There was a problem hiding this comment.
The thing about this is that it does change the slice order. If we care about order from a test assertion perspective, this will fail that assumption.
Tests should be written such that if the order is required, they should specify in the query though, so we can leave this.
There was a problem hiding this comment.
Good point! I think we will probably not have to worry about it since we do not rely on ordering of authentication codes (there is no method to get multiple codes right now either) but seems worth keeping in mind.
I lazily copied these lines from other implementations in this file but I think it might be better to add a remove and change all of these:
func remove[T any](slice []T, i int) []T {
return append(slice[:i], slice[i+1:]...)
}
There was a problem hiding this comment.
The way I have been doing it (for apps and secrets which do need ordering) is to sort them before returning in their Get* calls so the underlying slice can be in any order.
There was a problem hiding this comment.
Ooo 1.21 has a Delete for slices apparently.
Good question, do the dbauthz tests look insufficient? They test all the methods and that they return the right thing, I think (I might have the wrong impression on some of the things it does though.) There are also the coderd tests added in the next PR (#11778) that end up hitting these methods, maybe I made the wrong call to split the PR here. I can hold off merging this until both are good to merge. |
Those just test authz assertions but don't necessarily call the actual SQL queries.
That might not be a bad idea in case you need to make some changes here. |
Ohhhh I had the wrong impression, good to know. |
186 exists in main now.
5a662d0 to
7d10b9e
Compare
7d10b9e to
29a9e26
Compare
Split out to hopefully make reviewing and merging easier. Works in tandom with #11778.
Blocks: