Skip to content

Commit 00e5c58

Browse files
mayanayzamayanayza
committed
Split registration into two-step flow with OIDC auto-linking
- Split RegisterModal into email-first (3a) and password (3b) sub-steps - Add POST /api/auth/check-email endpoint for email availability check - Auto-link OIDC identity when email matches existing account instead of erroring, redirect existing users to app root instead of onboarding - Show inline error with "Sign in instead" link for duplicate emails - Fix OIDC error propagation: use error_code query param, read on mount - Fix small-screen modal layout with responsive padding and max-height - Bump anonymous rate limit from 20/min to 40/min (burst 5 -> 10) - Track onboarding_use_case_selected event in self-hosted flow - Clean up 17 unused i18n keys
1 parent 79cc650 commit 00e5c58

File tree

15 files changed

+676
-253
lines changed

15 files changed

+676
-253
lines changed

‎backend/src/server/auth/handlers.rs‎

Lines changed: 82 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ use crate::server::{
22
auth::{
33
r#impl::{
44
api::{
5-
ForgotPasswordRequest, LoginRequest, OidcAuthorizeParams, OidcCallbackParams,
6-
OnboardingNetworkState, OnboardingStateResponse, OnboardingStepRequest,
7-
RegisterRequest, ResendVerificationRequest, ResetPasswordRequest, SetupRequest,
8-
SetupResponse, UpdateEmailPasswordRequest, VerifyEmailRequest,
5+
CheckEmailRequest, ForgotPasswordRequest, LoginRequest, OidcAuthorizeParams,
6+
OidcCallbackParams, OnboardingNetworkState, OnboardingStateResponse,
7+
OnboardingStepRequest, RegisterRequest, ResendVerificationRequest,
8+
ResetPasswordRequest, SetupRequest, SetupResponse, UpdateEmailPasswordRequest,
9+
VerifyEmailRequest,
910
},
1011
base::{LoginRegisterParams, PendingNetworkSetup, PendingSetup},
1112
oidc::{OidcFlow, OidcPendingAuth, OidcProviderMetadata, OidcRegisterParams},
@@ -14,7 +15,7 @@ use crate::server::{
1415
auth::AuthenticatedEntity,
1516
permissions::{Authorized, IsUser},
1617
},
17-
oidc::OidcService,
18+
oidc::{OidcRegisterResult, OidcService},
1819
},
1920
config::{AppState, DeploymentType, get_deployment_type},
2021
daemon_api_keys::r#impl::base::{DaemonApiKey, DaemonApiKeyBase},
@@ -24,15 +25,18 @@ use crate::server::{
2425
shared::{
2526
events::types::{TelemetryEvent, TelemetryOperation},
2627
services::traits::CrudService,
28+
storage::filter::StorableFilter,
2729
storage::traits::Storable,
2830
types::api::{ApiError, ApiErrorResponse, ApiResponse, ApiResult, EmptyApiResponse},
31+
types::error_codes::ErrorCode,
2932
},
3033
snmp_credentials::r#impl::base::{SnmpCredential, SnmpCredentialBase, SnmpVersion},
3134
topology::types::base::{Topology, TopologyBase},
3235
users::r#impl::base::User,
3336
};
3437
use axum::{
3538
extract::{Path, Query, State},
39+
http::StatusCode,
3640
response::{Json, Redirect},
3741
routing::get,
3842
};
@@ -51,6 +55,7 @@ pub const DEMO_HOST: &str = "demo.scanopy.net";
5155

5256
pub fn create_router() -> OpenApiRouter<Arc<AppState>> {
5357
OpenApiRouter::new()
58+
.routes(routes!(check_email))
5459
.routes(routes!(register))
5560
.routes(routes!(login))
5661
.routes(routes!(logout))
@@ -70,6 +75,36 @@ pub fn create_router() -> OpenApiRouter<Arc<AppState>> {
7075
.routes(routes!(resend_verification))
7176
}
7277

78+
#[utoipa::path(
79+
post,
80+
path = "/check-email",
81+
tags = ["auth", "internal"],
82+
request_body = CheckEmailRequest,
83+
responses(
84+
(status = 200, description = "Email is available", body = EmptyApiResponse),
85+
(status = 409, description = "Email already in use", body = ApiErrorResponse),
86+
)
87+
)]
88+
async fn check_email(
89+
State(state): State<Arc<AppState>>,
90+
Json(request): Json<CheckEmailRequest>,
91+
) -> ApiResult<Json<ApiResponse<()>>> {
92+
let existing = state
93+
.services
94+
.user_service
95+
.get_all(StorableFilter::<User>::new_from_email(&request.email))
96+
.await?;
97+
if !existing.is_empty() {
98+
return Err(ApiError::coded(
99+
StatusCode::CONFLICT,
100+
ErrorCode::UserEmailInUse {
101+
email: request.email.to_string(),
102+
},
103+
));
104+
}
105+
Ok(Json(ApiResponse::success(())))
106+
}
107+
73108
#[utoipa::path(
74109
post,
75110
path = "/register",
@@ -117,6 +152,21 @@ async fn register(
117152
));
118153
}
119154

155+
// Check if email is already in use
156+
let existing = state
157+
.services
158+
.user_service
159+
.get_all(StorableFilter::<User>::new_from_email(&request.email))
160+
.await?;
161+
if !existing.is_empty() {
162+
return Err(ApiError::coded(
163+
StatusCode::CONFLICT,
164+
ErrorCode::UserEmailInUse {
165+
email: request.email.to_string(),
166+
},
167+
));
168+
}
169+
120170
let user_agent = user_agent.map(|u| u.to_string());
121171

122172
// Check for pending invite
@@ -1388,7 +1438,12 @@ async fn handle_register_flow(
13881438
)
13891439
.await
13901440
{
1391-
Ok(user) => {
1441+
Ok(result) => {
1442+
let (user, is_new_user) = match result {
1443+
OidcRegisterResult::NewUser(user) => (user, true),
1444+
OidcRegisterResult::ExistingUser(user) => (user, false),
1445+
};
1446+
13921447
// Cycle session ID to prevent session fixation attacks
13931448
if let Err(e) = session.cycle_id().await {
13941449
tracing::error!("Failed to cycle session ID: {}", e);
@@ -1409,35 +1464,48 @@ async fn handle_register_flow(
14091464
)));
14101465
}
14111466

1412-
// If this is a new org and setup was provided, apply it
1413-
if is_new_org {
1467+
// Only apply pending setup for new users in new orgs
1468+
if is_new_user && is_new_org {
14141469
if let Some(setup) = pending_setup
14151470
&& let Err(e) = apply_pending_setup(&state, &user, setup).await
14161471
{
14171472
tracing::error!("Failed to apply pending setup: {:?}", e);
14181473
// Don't fail registration, just log the error
14191474
// The user can complete onboarding manually
14201475
}
1421-
1422-
// Clear pending setup data from session
1423-
clear_pending_setup(&session).await;
14241476
}
14251477

1478+
// Clear pending setup data from session
1479+
clear_pending_setup(&session).await;
1480+
14261481
// Clear OIDC session data
14271482
let _ = session.remove::<OidcPendingAuth>("oidc_pending_auth").await;
14281483
let _ = session.remove::<String>("oidc_provider_slug").await;
14291484
let _ = session.remove::<String>("oidc_return_url").await;
14301485
let _ = session.remove::<bool>("oidc_terms_accepted").await;
14311486
let _ = session.remove::<bool>("oidc_marketing_opt_in").await;
14321487

1433-
Ok(Redirect::to(return_url.as_str()))
1488+
if is_new_user {
1489+
Ok(Redirect::to(return_url.as_str()))
1490+
} else {
1491+
// Existing user auto-logged in — send to app root, not onboarding
1492+
Ok(Redirect::to("/"))
1493+
}
14341494
}
14351495
Err(e) => {
14361496
tracing::error!("Failed to register via OIDC: {}", e);
1497+
let error_msg = format!("Failed to register: {}", e);
1498+
let err_str = e.to_string();
1499+
let error_code = if err_str.contains("already exists") {
1500+
"&error_code=user_email_in_use"
1501+
} else {
1502+
""
1503+
};
14371504
Err(Redirect::to(&format!(
1438-
"{}?error={}",
1505+
"{}?error={}{}",
14391506
return_url,
1440-
urlencoding::encode(&format!("Failed to register: {}", e))
1507+
urlencoding::encode(&error_msg),
1508+
error_code
14411509
)))
14421510
}
14431511
}

‎backend/src/server/auth/impl/api.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ fn validate_password_complexity(password: &str) -> Result<(), validator::Validat
4343
Ok(())
4444
}
4545

46+
/// Check email availability request
47+
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
48+
pub struct CheckEmailRequest {
49+
#[schema(value_type = String, format = "email")]
50+
pub email: EmailAddress,
51+
}
52+
4653
/// Session user info (stored in session, not in database)
4754
#[derive(Debug, Clone, Serialize, Deserialize)]
4855
pub struct SessionUser {

‎backend/src/server/auth/middleware/rate_limit.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ fn get_limiters() -> &'static RateLimiters {
5151
Quota::per_minute(NonZeroU32::new(300).unwrap())
5252
.allow_burst(NonZeroU32::new(150).unwrap()),
5353
)),
54-
// Anonymous/unauthenticated: 20 requests per minute with burst of 5
54+
// Anonymous/unauthenticated: 40 requests per minute with burst of 10
5555
anonymous: Arc::new(RateLimiter::keyed(
56-
Quota::per_minute(NonZeroU32::new(20).unwrap())
57-
.allow_burst(NonZeroU32::new(5).unwrap()),
56+
Quota::per_minute(NonZeroU32::new(40).unwrap())
57+
.allow_burst(NonZeroU32::new(10).unwrap()),
5858
)),
5959
// External services (Prometheus, etc.): 60 requests per minute with burst of 10
6060
// Sufficient for typical 15-30 second scrape intervals
@@ -160,16 +160,16 @@ fn check_anonymous(ip: IpAddr) -> Result<RateLimitInfo, RateLimitInfo> {
160160

161161
match limiters.anonymous.check_key(&key) {
162162
Ok(_) => Ok(RateLimitInfo {
163-
limit: 20,
164-
remaining: 19,
163+
limit: 40,
164+
remaining: 39,
165165
reset_in_secs: 60,
166166
}),
167167
Err(not_until) => {
168168
let wait_time = not_until
169169
.wait_time_from(DefaultClock::default().now())
170170
.as_secs();
171171
Err(RateLimitInfo {
172-
limit: 20,
172+
limit: 40,
173173
remaining: 0,
174174
reset_in_secs: wait_time,
175175
})

‎backend/src/server/auth/oidc.rs‎

Lines changed: 93 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ use crate::server::{
2828
users::{r#impl::base::User, service::UserService},
2929
};
3030

31+
/// Result of OIDC register — distinguishes new registration from auto-login of existing user
32+
pub enum OidcRegisterResult {
33+
/// Brand new user was created
34+
NewUser(User),
35+
/// Existing user was found and logged in (OIDC subject or email matched)
36+
ExistingUser(User),
37+
}
38+
3139
pub struct OidcService {
3240
providers: HashMap<String, Arc<OidcProvider>>,
3341
auth_service: Arc<AuthService>,
@@ -93,14 +101,14 @@ impl OidcService {
93101
self.providers.is_empty()
94102
}
95103

96-
/// Register new user via OIDC (fails if account already exists)
104+
/// Register new user via OIDC, or auto-login if account already exists
97105
pub async fn register(
98106
&self,
99107
pending_auth: OidcPendingAuth,
100108
params: LoginRegisterParams,
101109
oidc_register_params: OidcRegisterParams<'_>,
102110
pending_setup: Option<PendingSetup>,
103-
) -> Result<User> {
111+
) -> Result<OidcRegisterResult> {
104112
let OidcRegisterParams {
105113
provider_slug,
106114
code,
@@ -125,16 +133,32 @@ impl OidcService {
125133
// Exchange code for user info using provider
126134
let user_info = provider.exchange_code(code, &pending_auth).await?;
127135

128-
// Check if user already exists with this OIDC account
129-
if let Some(_existing_user) = self
136+
// If user already exists with this OIDC account, log them in
137+
if let Some(existing_user) = self
130138
.user_service
131139
.get_user_by_oidc(&user_info.subject)
132140
.await?
133141
{
134-
return Err(anyhow!(
135-
"An account with this {} login already exists. Please use the login flow instead.",
136-
provider.name
137-
));
142+
let authentication: AuthenticatedEntity = existing_user.clone().into();
143+
self.event_bus
144+
.publish_auth(AuthEvent {
145+
id: Uuid::new_v4(),
146+
user_id: Some(existing_user.id),
147+
organization_id: Some(existing_user.base.organization_id),
148+
timestamp: Utc::now(),
149+
operation: AuthOperation::LoginSuccess,
150+
ip_address: ip,
151+
user_agent,
152+
metadata: serde_json::json!({
153+
"method": "oidc",
154+
"provider": provider.slug,
155+
"provider_name": provider.name,
156+
"via_register_flow": true
157+
}),
158+
authentication,
159+
})
160+
.await?;
161+
return Ok(OidcRegisterResult::ExistingUser(existing_user));
138162
}
139163

140164
// Parse or create fallback email
@@ -154,6 +178,66 @@ impl OidcService {
154178
));
155179
}
156180

181+
// Check if email is already in use by another account
182+
let existing = self
183+
.user_service
184+
.get_all(
185+
crate::server::shared::storage::filter::StorableFilter::<User>::new_from_email(
186+
&email,
187+
),
188+
)
189+
.await?;
190+
if !existing.is_empty() {
191+
// Auto-link OIDC identity to existing account and log them in
192+
let mut existing_user = existing.into_iter().next().unwrap();
193+
194+
// If already linked to a different OIDC provider, don't override
195+
if let Some(existing_provider) = &existing_user.base.oidc_provider
196+
&& existing_provider != &provider.slug
197+
{
198+
let existing_provider_name = self
199+
.get_provider(existing_provider)
200+
.map(|p| p.name.as_str())
201+
.unwrap_or(existing_provider.as_str());
202+
return Err(anyhow!(
203+
"This account is already linked to {}. Please sign in with {} or unlink it first.",
204+
existing_provider_name,
205+
existing_provider_name
206+
));
207+
}
208+
209+
existing_user.base.oidc_provider = Some(provider.slug.clone());
210+
existing_user.base.oidc_subject = Some(user_info.subject);
211+
existing_user.base.oidc_linked_at = Some(chrono::Utc::now());
212+
213+
let authentication: AuthenticatedEntity = existing_user.clone().into();
214+
215+
self.event_bus
216+
.publish_auth(AuthEvent {
217+
id: Uuid::new_v4(),
218+
user_id: Some(existing_user.id),
219+
organization_id: Some(existing_user.base.organization_id),
220+
timestamp: Utc::now(),
221+
operation: AuthOperation::OidcLinked,
222+
ip_address: ip,
223+
user_agent,
224+
metadata: serde_json::json!({
225+
"method": "oidc",
226+
"provider": provider.slug,
227+
"provider_name": provider.name,
228+
"auto_linked": true
229+
}),
230+
authentication: authentication.clone(),
231+
})
232+
.await?;
233+
234+
let updated = self
235+
.user_service
236+
.update(&mut existing_user, authentication)
237+
.await?;
238+
return Ok(OidcRegisterResult::ExistingUser(updated));
239+
}
240+
157241
// Register new user
158242
let user = self
159243
.auth_service
@@ -195,7 +279,7 @@ impl OidcService {
195279
})
196280
.await?;
197281

198-
Ok(user)
282+
Ok(OidcRegisterResult::NewUser(user))
199283
}
200284

201285
/// Login existing user via OIDC (fails if account doesn't exist)

‎backend/tests/integration/infra.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ pub async fn setup_authenticated_user(client: &TestClient) -> Result<User, Strin
470470
println!("✅ Registered new user: {}", user.base.email);
471471
Ok(user)
472472
}
473-
Err(e) if e.contains("already taken") => {
473+
Err(e) if e.contains("already taken") || e.contains("already in use") => {
474474
println!("User already exists, logging in...");
475475
client.login(&test_email, TEST_PASSWORD).await
476476
}

0 commit comments

Comments
 (0)