-
Notifications
You must be signed in to change notification settings - Fork 1
Anonymous: Add configurable device limit #762
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
base: enhance-anonymous-access
Are you sure you want to change the base?
Changes from all commits
3647ba7
dfcf80a
564125d
2ed48e0
b9a9b93
f4d9992
59f0f1b
3dd39e5
930006b
b3497d7
14c9f3c
ab4e3df
637057e
01181a6
308fc2b
e5957f2
e985cb3
c123d06
2081caf
a6a8527
61ea278
0751aa3
a1eca1c
55b57e9
6c6e0f1
d5ab85c
252fa2d
4685e87
87ab6d9
2978805
5fe1cbf
b59f27d
973a527
158871c
d2b6fc6
db33559
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,21 +2,20 @@ package anonimpl | |
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "net/http" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/grafana/grafana/pkg/infra/log" | ||
| "github.com/grafana/grafana/pkg/services/anonymous" | ||
| "github.com/grafana/grafana/pkg/services/anonymous/anonimpl/anonstore" | ||
| "github.com/grafana/grafana/pkg/services/authn" | ||
| "github.com/grafana/grafana/pkg/services/org" | ||
| "github.com/grafana/grafana/pkg/setting" | ||
| ) | ||
|
|
||
| var _ authn.ContextAwareClient = new(Anonymous) | ||
|
|
||
| const timeoutTag = 2 * time.Minute | ||
|
|
||
| type Anonymous struct { | ||
| cfg *setting.Cfg | ||
| log log.Logger | ||
|
|
@@ -42,19 +41,13 @@ func (a *Anonymous) Authenticate(ctx context.Context, r *authn.Request) (*authn. | |
| httpReqCopy.RemoteAddr = r.HTTPRequest.RemoteAddr | ||
| } | ||
|
|
||
| go func() { | ||
| defer func() { | ||
| if err := recover(); err != nil { | ||
| a.log.Warn("Tag anon session panic", "err", err) | ||
| } | ||
| }() | ||
|
|
||
| newCtx, cancel := context.WithTimeout(context.Background(), timeoutTag) | ||
| defer cancel() | ||
| if err := a.anonDeviceService.TagDevice(newCtx, httpReqCopy, anonymous.AnonDeviceUI); err != nil { | ||
| a.log.Warn("Failed to tag anonymous session", "error", err) | ||
| if err := a.anonDeviceService.TagDevice(ctx, httpReqCopy, anonymous.AnonDeviceUI); err != nil { | ||
| if errors.Is(err, anonstore.ErrDeviceLimitReached) { | ||
| return nil, err | ||
| } | ||
| }() | ||
|
|
||
| a.log.Warn("Failed to tag anonymous session", "error", err) | ||
| } | ||
|
|
||
|
Comment on lines
+44
to
51
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cache inconsistency in TagDevice persists the device ID before database verification, allowing rejected devices to bypass limits on subsequent retries. Clear the cache entry using a.localCache.Delete(key) in the tagDeviceUI error path. Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| return &authn.Identity{ | ||
| ID: authn.AnonymousNamespaceID, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Time-of-Check to Time-of-Use (TOCTOU) race condition between CountDevices and the device insert allows concurrent requests to bypass limit checks and exceed the configured device count. Enforce the limit atomically using a database constraint or lock.
Prompt for LLM
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.