feat: `createAzureAdOAuthConfig()` and `createAzureAdb2cOAuthConfig()` #293

mcgear · 2024-01-13T09:12:29Z

This would address #115 , i believe i have it working. Haven't tested a lot of different scopes, but it does work with the scopes we are using https://management.core.windows.net//.default and openid.

Here is an example of creating it:

createAzureOAuthConfig({
   scope: ["openid"],
 }),

You can also change the prefix used on the environment variables from the default AZURE to whatever is needed:

createAzureOAuthConfig({
   envPrefix: "MY_PREFIX",
   scope: ["openid"],
 }),

Update to latest

mcgear · 2024-01-13T09:32:28Z

Still working on some issues with azure ADB2C support involving some query string params that are needed for that, and proper redirect

mcgear · 2024-01-13T11:54:48Z

Support for Azure and ADB2C are working

mcgear · 2024-01-14T16:26:44Z

To use this until it gets merged, you can use an import map like this:

    "$fresh/oauth": "https://raw.githubusercontent.com/fathym-deno/deno_kv_oauth/main/mod.ts",

iuioiua

Sorry for the delay and thanks for the PR! It'll be good to have this functionality added. A few suggestions, mostly around sticking to how other createXOAuthConfig() functions are implemented.

lib/sign_in.ts

lib/handle_callback.ts

iuioiua · 2024-01-15T22:27:07Z

lib/create_azure_oauth_config.ts

+ */
+export function createAzureOAuthConfig(config: {
+  /** Used to set the prefix used to access environment variables; defaults to `AZURE`.*/
+  envPrefix?: string;


This option doesn't exist for any of the other platform config creator functions. Can you please remove it? A single, hardcoded environment variable prefix was a deliberate design decision to simplify logic.

Is there anyway we can include this change? A few reasons, 1) I was going to create a pr to add it to the other ones. When doing the ahead of time builds with the github oauth provider, this fails as github actions do not allow environment variables that start with GITHUB_. 2) by design, and due to azure capabilities, we have to use two separate Azure providers. One for oauth provider, and the other for access to azure infrastructure via oath access tokens. 3) The default env tokens we would use clash with existing environment variables that are used, and established, in this and other projects. I imagine that other users will eventually run into this clash when adding this library in the future.

I also tried to keep it inline with other impls by defaulting to the singular AZURE suffix to keep inline when envPrefix is omitted.

Let's hardcode the environment variable prefix for now, like for every other provider. We've kept this API's library simple and consistent while working for most use cases. Edge cases can be handled by using custom OAuth configs.

iuioiua · 2024-01-15T22:28:17Z

lib/create_azure_oauth_config.ts

+ *
+ * @see {@link https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc}
+ */
+export function createAzureOAuthConfig(config: {


This function differs in its logic from other similar functions within this library. For ease of management of use, could you please try following the implementation of other createXOAuthConfig() functions in the library?

The two major differences i see are 1) related to the logic around envPrefix, hoping we can include that as i'm not sure how i would support our need for multiple Azure oAuth providers without, and 2) around the ClientID logic. For the client ID logic, with ClientID being required in scopes for the oAuth flow to work, i figured it would be best to add in the createXOauthConfig so that users did not have to try and access additional environment variables during setup of the OAuth config so that user configuration was more closely related to other createXOAuthConfig methods. Would the preference be that we as maintainers deal with this nuance in code? Or that we push it on the user to need to access the environment variable during setup, diverging from how the other ones work?

iuioiua

PTAL at my suggested implementation. Can you please also add a test to lib/create_oauth_config_test.ts? You may need to add the following to the test:

// For Azure
Deno.env.set(`${envPrefix}_TENANT_ID`, crypto.randomUUID());

Regarding multiple providers using for the same platform, let's open a separate issue to discuss. I feel like such a use might just warrant a custom OAuth configuration.

iuioiua · 2024-01-18T06:36:11Z

lib/create_azure_oauth_config.ts

+/**
+ * Returns the OAuth configuration for Azure.
+ *
+ * Requires `--allow-env[=AZURE_CLIENT_ID,AZURE_CLIENT_SECRET,AZURE_CLOUD_INSTANCE,AZURE_TENANT_ID]`
+ * permissions and environment variables:
+ * 1. `AZURE_CLIENT_ID`
+ * 2. `AZURE_CLIENT_SECRET`
+ * 3. `AZURE_CLOUD_INSTANCE`
+ * 4. `AZURE_TENANT_ID`
+ *
+ * @example
+ * ```ts
+ * import { createAzureOAuthConfig } from "https://deno.land/x/deno_kv_oauth@$VERSION/mod.ts";
+ *
+ * const oauthConfig = createAzureOAuthConfig({
+ *   envPrefix: "AZURE",
+ *   redirectUri: "http://localhost:8000/callback",
+ *   scope: "openid",
+ * });
+ * ```
+ *
+ * You can use a separate environment variable prefix with the envPrefix configureation.
+ * Requires `--allow-env[=MY_PREFIX_CLIENT_ID,MY_PREFIX_CLIENT_SECRET,MY_PREFIX_CLOUD_INSTANCE,MY_PREFIX_TENANT_ID]`
+ * permissions and environment variables:
+ * 1. `MY_PREFIX_CLIENT_ID`
+ * 2. `MY_PREFIX_CLIENT_SECRET`
+ * 3. `MY_PREFIX_CLOUD_INSTANCE`
+ * 4. `MY_PREFIX_TENANT_ID`
+ *
+ * @example
+ * ```ts
+ * import { createAzureOAuthConfig } from "https://deno.land/x/deno_kv_oauth@$VERSION/mod.ts";
+ *
+ * const oauthConfig = createAzureOAuthConfig({
+ *   envPrefix: "MY_PREFIX",
+ *   redirectUri: "http://localhost:8000/callback",
+ *   scope: "openid",
+ * });
+ * ```
+ *
+ * This also supports the ability to connect with Azure AD B2C instances using the
+ * proper configurations for AD B2C instances
+ *
+ * @see {@link https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc}
+ */
+export function createAzureOAuthConfig(config: {
+  /** Used to set the prefix used to access environment variables; defaults to `AZURE`.*/
+  envPrefix?: string;
+  /** @see {@linkcode OAuth2ClientConfig.redirectUri} */
+  redirectUri?: string;
+  /** @see {@linkcode OAuth2ClientConfig.defaults.scope} */
+  scope: string | string[];
+}): OAuth2ClientConfig {
+  const getEnvKey = (key: string) => {
+    return `${config.envPrefix || "AZURE"}_${key}`;
+  };
+
+  let cloudInstance = getRequiredEnv(getEnvKey("CLOUD_INSTANCE"));
+
+  if (!cloudInstance.endsWith("/")) {
+    cloudInstance = `${cloudInstance}/`;
+  }
+
+  const tenantId = getRequiredEnv(getEnvKey("TENANT_ID"));
+
+  const policy = Deno.env.get(getEnvKey("POLICY"));
+
+  const path = policy ? `${tenantId}/${policy}` : tenantId;
+
+  const baseURL = `${cloudInstance}${path}/oauth2`;
+
+  const clientId = getRequiredEnv(getEnvKey("CLIENT_ID"));
+
+  if (Array.isArray(config.scope) && config.scope.some((s) => s === "openid")) {
+    config.scope.push(clientId);
+  } else if (
+    typeof config.scope === "string" &&
+    config.scope.includes("openid")
+  ) {
+    config.scope = `${config.scope} ${clientId}`;
+  }
+
+  return {
+    clientId,
+    clientSecret: getRequiredEnv(getEnvKey("CLIENT_SECRET")),
+    authorizationEndpointUri: `${baseURL}/v2.0/authorize`,
+    tokenUri: `${baseURL}/v2.0/token`,
+    redirectUri: config.redirectUri,
+    defaults: {
+      scope: config.scope,
+    },
+  };
+}


Suggested change

/**

* Returns the OAuth configuration for Azure.

*

* Requires `--allow-env[=AZURE_CLIENT_ID,AZURE_CLIENT_SECRET,AZURE_CLOUD_INSTANCE,AZURE_TENANT_ID]`

* permissions and environment variables:

* 1. `AZURE_CLIENT_ID`

* 2. `AZURE_CLIENT_SECRET`

* 3. `AZURE_CLOUD_INSTANCE`

* 4. `AZURE_TENANT_ID`

*

* @example

* ```ts

* import { createAzureOAuthConfig } from "https://deno.land/x/deno_kv_oauth@$VERSION/mod.ts";

*

* const oauthConfig = createAzureOAuthConfig({

* envPrefix: "AZURE",

* redirectUri: "http://localhost:8000/callback",

* scope: "openid",

* });

* ```

*

* You can use a separate environment variable prefix with the envPrefix configureation.

* Requires `--allow-env[=MY_PREFIX_CLIENT_ID,MY_PREFIX_CLIENT_SECRET,MY_PREFIX_CLOUD_INSTANCE,MY_PREFIX_TENANT_ID]`

* permissions and environment variables:

* 1. `MY_PREFIX_CLIENT_ID`

* 2. `MY_PREFIX_CLIENT_SECRET`

* 3. `MY_PREFIX_CLOUD_INSTANCE`

* 4. `MY_PREFIX_TENANT_ID`

*

* @example

* ```ts

* import { createAzureOAuthConfig } from "https://deno.land/x/deno_kv_oauth@$VERSION/mod.ts";

*

* const oauthConfig = createAzureOAuthConfig({

* envPrefix: "MY_PREFIX",

* redirectUri: "http://localhost:8000/callback",

* scope: "openid",

* });

* ```

*

* This also supports the ability to connect with Azure AD B2C instances using the

* proper configurations for AD B2C instances

*

* @see {@link https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc}

*/

export function createAzureOAuthConfig(config: {

/** Used to set the prefix used to access environment variables; defaults to `AZURE`.*/

envPrefix?: string;

/** @see {@linkcode OAuth2ClientConfig.redirectUri} */

redirectUri?: string;

/** @see {@linkcode OAuth2ClientConfig.defaults.scope} */

scope: string | string[];

}): OAuth2ClientConfig {

const getEnvKey = (key: string) => {

return `${config.envPrefix || "AZURE"}_${key}`;

};

let cloudInstance = getRequiredEnv(getEnvKey("CLOUD_INSTANCE"));

if (!cloudInstance.endsWith("/")) {

cloudInstance = `${cloudInstance}/`;

}

const tenantId = getRequiredEnv(getEnvKey("TENANT_ID"));

const policy = Deno.env.get(getEnvKey("POLICY"));

const path = policy ? `${tenantId}/${policy}` : tenantId;

const baseURL = `${cloudInstance}${path}/oauth2`;

const clientId = getRequiredEnv(getEnvKey("CLIENT_ID"));

if (Array.isArray(config.scope) && config.scope.some((s) => s === "openid")) {

config.scope.push(clientId);

} else if (

typeof config.scope === "string" &&

config.scope.includes("openid")

) {

config.scope = `${config.scope} ${clientId}`;

}

return {

clientId,

clientSecret: getRequiredEnv(getEnvKey("CLIENT_SECRET")),

authorizationEndpointUri: `${baseURL}/v2.0/authorize`,

tokenUri: `${baseURL}/v2.0/token`,

redirectUri: config.redirectUri,

defaults: {

scope: config.scope,

},

};

}

/**

* Returns the OAuth configuration for Azure.

*

* Requires `--allow-env[=AZURE_CLIENT_ID,AZURE_CLIENT_SECRET,AZURE_TENANT_ID]`

* permissions and environment variables:

* 1. `AZURE_CLIENT_ID`

* 2. `AZURE_CLIENT_SECRET`

* 3. `AZURE_TENANT_ID`

*

* @example

* ```ts

* import { createAzureOAuthConfig } from "https://deno.land/x/deno_kv_oauth@$VERSION/mod.ts";

*

* const oauthConfig = createAzureOAuthConfig({

* redirectUri: "http://localhost:8000/callback",

* scope: "https://graph.microsoft.com/User.Read",

* });

* ```

*

* @see {@link https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-auth-code-flow}

*/

export function createAzureOAuthConfig(config: {

/** @see {@linkcode OAuth2ClientConfig.redirectUri} */

redirectUri?: string;

/** @see {@linkcode OAuth2ClientConfig.defaults.scope} */

scope: string | string[];

}): OAuth2ClientConfig {

const baseUrl = `https://login.microsoftonline.com/${

getRequiredEnv("AZURE_TENANT_ID")

}/oauth2/v2.0`;

return {

clientId: getRequiredEnv("AZURE_CLIENT_ID"),

clientSecret: getRequiredEnv("AZURE_CLIENT_SECRET"),

authorizationEndpointUri: `${baseUrl}/authorize`,

tokenUri: `${baseUrl}/token`,

redirectUri: config.redirectUri,

defaults: {

scope: config.scope,

},

};

}

These changes bring the Azure implementation into line with the other platforms. I've tested it, and it works as expected. I want to clarify that these functions are meant to cater to the majority of use cases, not all use cases. Advanced use cases can take advantage of custom OAuth configs, if needed.

With these changes azure ADB2C won't work, which is actually what we need to use, the major issue being the hardcoding of https://login.microsoftonline.com. I split AD and ADB2C into separate config providers. Though i still think we should include the logic around client ID, otherwise the end user experience is more complicated by having to know to add the client ID as a scope instead of just openid.

Let me know if you want me to remove the client ID logic.

Tests were added as well.

mcgear · 2024-01-19T16:49:23Z

PTAL at my suggested implementation. Can you please also add a test to lib/create_oauth_config_test.ts? You may need to add the following to the test:
// For Azure
Deno.env.set(`${envPrefix}_TENANT_ID`, crypto.randomUUID());
Regarding multiple providers using for the same platform, let's open a separate issue to discuss. I feel like such a use might just warrant a custom OAuth configuration.

With the separation of Azure ADB2C from Azure AD, this would work for us now.

iuioiua · 2024-01-29T22:20:06Z

Apologies for the delay, @mcgear. I don't like to drag PRs on for this long. I'll review this PR by the end of this week.

mcgear · 2024-01-30T17:00:10Z

Sounds good, i'll try to be quick on incorporating any other changes, but i think i got both of the new ones inline with your other comments. The only real last question is whether or not we keep the ClientID code in or force end users to add it themselves when leveraging openid

iuioiua · 2024-01-31T06:46:23Z

The user should add the client ID if they want to leverage OpenID. Looking at the current implementations, they still include logic that isn't present for other platforms. Again, can you please remove this logic?

mcgear · 2024-01-31T18:04:17Z

Those changes are in

iuioiua

This is looking much better! Can you please provide ensure you've followed these steps in this PR so we know the client functions are working?

iuioiua · 2024-02-01T06:30:03Z

README.md

@@ -314,6 +314,7 @@ is set in the following order of precedence:
 The following providers have pre-defined OAuth configurations:

 1. [Auth0](https://deno.land/x/deno_kv_oauth/mod.ts?s=createAuth0OAuthConfig)
+1. [Azure](https://deno.land/x/deno_kv_oauth/mod.ts?s=createAzureOAuthConfig)


Can you please update this with the two new functions?

Fixed the readme

mcgear · 2024-02-01T18:17:19Z

This is looking much better! Can you please provide ensure you've followed these steps in this PR so we know the client functions are working?

2 and 3 from that link are taken care of, i'm not sure what to do for 1 as the link on contributing is broken. I am using both configurations in a deployment, and they are working. Let me know what sort of screenshot i should be providing.

iuioiua · 2024-02-01T21:16:40Z

lib/create_azure_adb2c_oauth_config.ts

+ *
+ * @see {@link https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc}
+ */
+export function createAzureADB2COAuthConfig(config: {


Suggested change

export function createAzureADB2COAuthConfig(config: {

export function createAzureAdb2cOAuthConfig(config: {

Ditto

iuioiua · 2024-02-01T21:17:09Z

lib/create_azure_ad_oauth_config.ts

+ *
+ * @see {@link https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc}
+ */
+export function createAzureADOAuthConfig(config: {


Suggested change

export function createAzureADOAuthConfig(config: {

export function createAzureAdOAuthConfig(config: {

Nit: the Deno naming convention doesn't capitalize abbreviations.

iuioiua · 2024-02-01T21:28:00Z

This is looking much better! Can you please provide ensure you've followed these steps in this PR so we know the client functions are working?

2 and 3 from that link are taken care of, i'm not sure what to do for 1 as the link on contributing is broken. I am using both configurations in a deployment, and they are working. Let me know what sort of screenshot i should be providing.

I've now corrected those links. Check it out now. It's about the screenshot of the demo working locally for you, so we know that the function works correctly. You may need to blur out any confidential text in this case.

mcgear · 2024-02-02T23:01:29Z

Date night tonight, but i should be able to get this worked up over the weekend so we can close this out.

mcgear · 2024-02-03T17:25:56Z

Alright, i think we are pretty close here:

iuioiua

LGTM! Excellent work. Thank you very much for this. It's a welcome addition.

mcgear and others added 4 commits January 13, 2024 00:46

Merge pull request #1 from denoland/main

2b05c78

Update to latest

Working azure oauth

99ef35e

Formatting

6098380

Support for cloud instance without trailing /

5b79392

mcgear added 4 commits January 13, 2024 02:33

Formatting

5d2d52f

working login for adb2c

596f2f5

Formatting

7de4bff

handle string scope case

af50abc

openid logic around adding app scope

48baba8

iuioiua requested changes Jan 15, 2024

View reviewed changes

mcgear added 5 commits January 16, 2024 09:35

revert formatting changes

5ed5475

fix formatting

9ee5975

Remove commented line

21bd64f

remove a helper to keep inline with other impls

dc896de

formatting

944c000

iuioiua changed the title ~~Azure OAuth~~ feat: createAzureOAuthConfig() Jan 18, 2024

iuioiua requested changes Jan 18, 2024

View reviewed changes

mcgear added 4 commits January 19, 2024 09:26

adjusted to hard code env variables and add provider for ADB2C

ba06d8d

Some adjustments based on comments

ce90f90

Added tests and got inline with other impls

bf46c47

Formatting

7b52ba2

Merge branch 'main' into main

dfaa4b3

Merge branch 'main' into main

1863aa0

mcgear added 3 commits January 31, 2024 10:47

remove client logic

6c84da9

Merge branch 'main' of https://github.com/fathym-deno/deno_kv_oauth

2090a5a

formatting

6b9fbec

iuioiua requested changes Feb 1, 2024

View reviewed changes

mcgear added 2 commits February 1, 2024 11:12

fix readme

5343028

Fixed examples

7e7f8c8

Fixed examples

8f47471

iuioiua requested changes Feb 1, 2024

View reviewed changes

mcgear and others added 5 commits February 2, 2024 16:01

Merge branch 'main' into main

6530e2a

change casing

0bfb994

Merge branch 'main' of https://github.com/fathym-deno/deno_kv_oauth

f3d890c

Fix example on Azure AD

1b91636

revert demo

fbf0cfe

Formatting

7e928ca

iuioiua changed the title ~~feat: createAzureOAuthConfig()~~ feat: createAzureAdOAuthConfig() and createAzureAdb2cOAuthConfig() Feb 5, 2024

iuioiua approved these changes Feb 5, 2024

View reviewed changes

iuioiua merged commit 4c91cc4 into denoland:main Feb 5, 2024
8 checks passed

iuioiua mentioned this pull request Feb 5, 2024

suggestion: add Azure (Microsoft) provider #115

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `createAzureAdOAuthConfig()` and `createAzureAdb2cOAuthConfig()` #293

feat: `createAzureAdOAuthConfig()` and `createAzureAdb2cOAuthConfig()` #293

mcgear commented Jan 13, 2024 •

edited

Loading

mcgear commented Jan 13, 2024

mcgear commented Jan 13, 2024

mcgear commented Jan 14, 2024

iuioiua left a comment •

edited

Loading

iuioiua Jan 15, 2024

mcgear Jan 16, 2024 •

edited

Loading

mcgear Jan 16, 2024

iuioiua Jan 18, 2024

iuioiua Jan 15, 2024

mcgear Jan 16, 2024

iuioiua left a comment

iuioiua Jan 18, 2024

mcgear Jan 19, 2024

mcgear Jan 19, 2024

mcgear commented Jan 19, 2024

iuioiua commented Jan 29, 2024

mcgear commented Jan 30, 2024

iuioiua commented Jan 31, 2024

mcgear commented Jan 31, 2024

iuioiua left a comment

iuioiua Feb 1, 2024

mcgear Feb 1, 2024

mcgear commented Feb 1, 2024 •

edited

Loading

iuioiua Feb 1, 2024

mcgear Feb 3, 2024

iuioiua Feb 1, 2024

mcgear Feb 3, 2024

iuioiua commented Feb 1, 2024

mcgear commented Feb 2, 2024

mcgear commented Feb 3, 2024

iuioiua left a comment

	export function createAzureADB2COAuthConfig(config: {
	export function createAzureAdb2cOAuthConfig(config: {

	export function createAzureADOAuthConfig(config: {
	export function createAzureAdOAuthConfig(config: {

feat: createAzureAdOAuthConfig() and createAzureAdb2cOAuthConfig() #293

feat: createAzureAdOAuthConfig() and createAzureAdb2cOAuthConfig() #293

Conversation

mcgear commented Jan 13, 2024 • edited Loading

mcgear commented Jan 13, 2024

mcgear commented Jan 13, 2024

mcgear commented Jan 14, 2024

iuioiua left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcgear Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iuioiua left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcgear commented Jan 19, 2024

iuioiua commented Jan 29, 2024

mcgear commented Jan 30, 2024

iuioiua commented Jan 31, 2024

mcgear commented Jan 31, 2024

iuioiua left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcgear commented Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iuioiua commented Feb 1, 2024

mcgear commented Feb 2, 2024

mcgear commented Feb 3, 2024

iuioiua left a comment

Choose a reason for hiding this comment

feat: `createAzureAdOAuthConfig()` and `createAzureAdb2cOAuthConfig()` #293

feat: `createAzureAdOAuthConfig()` and `createAzureAdb2cOAuthConfig()` #293

mcgear commented Jan 13, 2024 •

edited

Loading

iuioiua left a comment •

edited

Loading

mcgear Jan 16, 2024 •

edited

Loading

mcgear commented Feb 1, 2024 •

edited

Loading