feat: add M365 one-click login link#46
Conversation
|
Warning Review limit reached
More reviews will be available in 48 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Microsoft 365 authentication helpers, specifically adding a command to generate a one-click, read-only login link (auth m365 login-link). It implements the backend broker session logic, including PKCE state generation, URL validation, and an in-memory session store (MemoryBrokerStore) to manage these sessions. Feedback was provided regarding the MemoryBrokerStore implementation, pointing out that a zero-value initialization could lead to a runtime panic due to a nil map. It is recommended to lazily initialize the sessions map inside the Save method under the lock to ensure resilience.
| func (s *MemoryBrokerStore) Save(_ context.Context, session BrokerSession) error { | ||
| state := strings.TrimSpace(session.State) | ||
| if state == "" { | ||
| return ErrBrokerStateNotFound | ||
| } | ||
|
|
||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
|
|
||
| s.sessions[state] = session | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
The MemoryBrokerStore struct can be initialized as a zero-value (e.g., var store MemoryBrokerStore or store := &MemoryBrokerStore{}). If this happens, s.sessions will be nil, and calling Save will result in a runtime panic when writing to the map.
To make the store resilient to zero-value initialization, we should lazily initialize the sessions map inside the Save method under the lock.
func (s *MemoryBrokerStore) Save(_ context.Context, session BrokerSession) error {
state := strings.TrimSpace(session.State)
if state == "" {
return ErrBrokerStateNotFound
}
s.mu.Lock()
defer s.mu.Unlock()
if s.sessions == nil {
s.sessions = make(map[string]BrokerSession)
}
s.sessions[state] = session
return nil
}There was a problem hiding this comment.
Fixed in 2ba3d21: Save now lazily initializes the session map under the lock, and TestMemoryBrokerStoreZeroValueSaveInitializesSessionMap covers zero-value usage. Local gates and CI are green.
2 participants
Summary
Implements the first Workit-side slice of #45: generating a Bernardo-safe, one-click Microsoft 365 read-only login link and the broker primitives needed by a hosted callback service.
This does not claim a hosted
login.workit.aiservice exists yet. It creates the CLI/API contract the service can call/use next.What changed
wk auth m365 login-link <email>https://login.workit.ai/m365/start/<opaque-state>S256User.Read,Mail.Read,Calendars.Readoffline_accessfor refresh tokenSafety
code_verifierin JSON output.Verification
make lint make deadcode go test ./... make coverage COVERAGE_MIN=70Result:
Dogfood:
Confirmed:
login_urlis the one-click broker URL.login.microsoftonline.com.redirect_uriis the supplied HTTPS broker callback.code_challenge_method=S256.Mail.SendorCalendars.ReadWritescopes.Refs #45: Workit now has the link/session contract; next PR can wire a hosted broker HTTP service/deployment.