Skip to content
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

auth: Fixes "Cannot read properties of undefined 'tenantId' error #1856

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

motm32
Copy link
Contributor

@motm32 motm32 commented Dec 12, 2024

Please refer to this rg issue for the error.

Also added the account property to isSignedIn() because for some reason I kept getting all of my tenants showing up in the list when I should only be getting unauthenticated tenants.

@motm32 motm32 requested a review from a team as a code owner December 12, 2024 21:33
@alexweininger
Copy link
Member

If the problem was because we weren't passing account into isSignedIn on line 16, then would this solve it?

image

I added the account property to our tenant interface recently #1849

@motm32
Copy link
Contributor Author

motm32 commented Dec 13, 2024

If the problem was because we weren't passing account into isSignedIn on line 16, then would this solve it?

image I added the account property to our tenant interface recently #1849

Yes lol for some reason it wasn't showing up when I first implemented it 😅

alexweininger
alexweininger previously approved these changes Dec 13, 2024
@@ -13,9 +13,12 @@ export async function getUnauthenticatedTenants(subscriptionProvider: AzureSubsc
const tenants = await subscriptionProvider.getTenants();
const unauthenticatedTenants: TenantIdDescription[] = [];
for await (const tenant of tenants) {
if (!await subscriptionProvider.isSignedIn(tenant.tenantId)) {
unauthenticatedTenants.push(tenant);
if (tenant) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if statement is weird to me but I understand why you included it. Maybe add a comment for future us so we're not confused about why it's there and accidentally remove it.

@motm32 motm32 merged commit 3cf2916 into main Dec 17, 2024
4 checks passed
@motm32 motm32 deleted the meganmott/fix-966 branch December 17, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants