From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id AC7FE1FF0C1 for ; Wed, 26 Aug 2026 14:49:48 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 4F8B32133F; Wed, 26 Aug 2026 14:49:48 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 26 Aug 2026 14:49:43 +0200 Message-Id: From: "Shan Shaji" To: "Azharul Haque" , Subject: Re: [PATCH dart-api-client v3 2/2] fix #4281: access: add OpenID Connect auth-url/login helpers X-Mailer: aerc 0.20.0 References: <20260810144713.75806-1-haque@azharul.com> <20260821034147.30194-1-haque@azharul.com> <20260821034147.30194-3-haque@azharul.com> In-Reply-To: <20260821034147.30194-3-haque@azharul.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787748575988 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.369 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: R345JWGZIWVTI5JIOWTBLEQ32DYILHNZ X-Message-ID-Hash: R345JWGZIWVTI5JIOWTBLEQ32DYILHNZ X-MailFrom: s.shaji@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hi Azharul, Thanks for factoring out the block. IMHO, we could separate the addition of helper and openID functions into two patches. 1. Factor out the helper. 2. Add the OpenID specific functions. On Fri Aug 21, 2026 at 5:41 AM CEST, Azharul Haque wrote: > Add openIdAuthUrl() and openIdLogin(), mirroring the existing > authenticate()/accessDomains() functions used by the login form > before an authenticated ProxmoxApiClient exists. > > openIdAuthUrl() requests the provider's authorization URL for a > realm from /access/openid/auth-url. openIdLogin() exchanges the > state/code obtained from the provider's redirect for a PVE ticket > via /access/openid/login, the same way authenticate() does for > password realms. > > The OpenID login response carries the authenticated username in its > body rather than it being known upfront by the caller, so > handleOpenIdLoginResponse() is added alongside the existing > handleAccessTicketResponse() to build Credentials from it. Its ticket > regex only matches PVE tickets (not PMG), since OpenID login is only > ever performed against a PVE realm; because the two regexes therefore > differ, the regex match stays in each handler while the shared tail > (deriving the expiration time and detecting an accompanying TFA > challenge) is factored into a common helper used by both. > > Suggested-by: Shan Shaji > Signed-off-by: Azharul Haque > --- [snip] > Future> accessDomains( > Uri apiBaseUrl, > bool validateSSL, { > diff --git a/lib/src/handle_ticket_response.dart b/lib/src/handle_ticket_= response.dart > index ba2128f..f4bffde 100644 > --- a/lib/src/handle_ticket_response.dart > +++ b/lib/src/handle_ticket_response.dart > @@ -5,6 +5,29 @@ import 'package:proxmox_dart_api_client/src/credentials.= dart'; > import 'package:proxmox_dart_api_client/src/extentions.dart'; > import 'package:proxmox_dart_api_client/src/tfa_challenge.dart'; > > +/// Shared tail of parsing a ticket response: derives the ticket's > +/// expiration time from its embedded timestamp, and determines whether = a > +/// TFA challenge accompanies it (either encoded in the ticket itself, o= r > +/// flagged via the legacy `NeedTFA` field). > +({DateTime expiration, TfaChallenge? tfa}) _parseTicketExpirationAndTfa( > + RegExpMatch ticketMatch, > + String ticket, > + Map bodyJson, > +) { > + final expiration =3D DateTime.fromMillisecondsSinceEpoch( > + int.parse(ticketMatch.group(3)!, radix: 16) * 1000); > + > + TfaChallenge? tfa; > + if (ticket.startsWith('PVE:!tfa!')) { > + tfa =3D TfaChallenge.fromJson( > + jsonDecode(Uri.decodeComponent(ticket.substring(9).split(':')[0]= ))); > + } else if (bodyJson['NeedTFA'] !=3D null && bodyJson['NeedTFA'] =3D=3D= 1) { > + tfa =3D TfaChallenge.legacy(); > + } > + > + return (expiration: expiration, tfa: tfa); > +} > + > Credentials handleAccessTicketResponse( > http.Response response, Credentials unauthenticatedCredentials) { > response.validate(false); > @@ -15,27 +38,46 @@ Credentials handleAccessTicketResponse( > > final csrfToken =3D bodyJson['CSRFPreventionToken']; > > - final ticketRegex =3D RegExp(r'(PVE|PMG)(?:QUAR)?:(?:(\S+):)?([A-Z0-9]= {8})::') > - .firstMatch(bodyJson['ticket'])!; > + final ticketMatch =3D RegExp(r'(PVE|PMG)(?:QUAR)?:(?:(\S+):)?([A-Z0-9]= {8})::') > + .firstMatch(ticket)!; > > - final time =3D DateTime.fromMillisecondsSinceEpoch( > - int.parse(ticketRegex.group(3)!, radix: 16) * 1000); > - > - TfaChallenge? tfa; > - if (ticket.startsWith('PVE:!tfa!')) { > - tfa =3D TfaChallenge.fromJson( > - jsonDecode(Uri.decodeComponent(ticket.substring(9).split(':')[0]= ))); > - } else if (bodyJson['NeedTFA'] !=3D null && bodyJson['NeedTFA'] =3D=3D= 1) { > - tfa =3D TfaChallenge.legacy(); > - } > + final parsed =3D _parseTicketExpirationAndTfa(ticketMatch, ticket, bod= yJson); > > return Credentials( > unauthenticatedCredentials.apiBaseUrl, > unauthenticatedCredentials.username, > ticket: ticket, > csrfToken: csrfToken, > - expiration: time, > - tfa: tfa, > + expiration: parsed.expiration, > + tfa: parsed.tfa, > + ); > +} > + Above changes could itself be another preparatory patch. [snip]