wip: demo that uses Looker as an OAUTH provider for private embed #218

bryans99 · 2025-05-22T23:17:11Z

This is very much a work in progress. Don't know when I will be able to finish it.

gemini-code-assist

Hello @bryans99, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! gemini-code-assist here, providing a summary of this pull request. This PR is marked as a work-in-progress and aims to demonstrate how to use Looker as an OAuth provider for private embedding scenarios. The primary changes involve adding client-side logic to initiate an OAuth PKCE (Proof Key for Code Exchange) flow from the demo page and integrating a button to trigger this flow. It also adjusts the embed configuration to not rely on the default Looker login screen.

Highlights

OAuth PKCE Flow Initiation: Adds JavaScript functions (oauth_login, secure_random, sha256_hash, array_to_hex) to demo/demo_single_frame.ts to generate a code verifier and challenge, construct the OAuth authorization URL with PKCE parameters, and redirect the user's browser to the Looker auth endpoint. This logic is triggered by a new button.
UI Integration: A new button with the ID oauth-login is added to demo/index.html. The initializeLoginControl function is added and called in demo/demo_single_frame.ts to attach a click event listener to this button, executing the oauth_login function.
Embed Configuration Adjustment: The .withAllowLoginScreen() method call is commented out in the createEmbed function in demo/demo_single_frame.ts. This change is likely made because the PR intends to handle authentication via the external OAuth flow rather than relying on the embed SDK to display the Looker login page.

Changelog

demo/demo_single_frame.ts
- Added oauth_login function (starts around line 528) to initiate the OAuth PKCE flow by redirecting the user to the Looker authorization endpoint with necessary parameters (client_id, redirect_uri, scope, state, code_challenge_method, code_challenge).
- Added helper functions (array_to_hex, secure_random, sha256_hash) (starts around line 545) to generate random strings and compute the SHA-256 hash required for the PKCE flow.
- Added initializeLoginControl function (starts around line 565) to find the 'oauth-login' button and attach a click event listener that calls oauth_login.
- Called initializeLoginControl within the main initializeControls function (around line 584) to ensure the login button is initialized on page load.
- Commented out the .withAllowLoginScreen() call (around line 699) in the createEmbed function.
demo/index.html
- Added a new <button> element with the ID oauth-login and class small-round (around line 40) within the controls section.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

OAuth flow begins,
Code verifier, challenge made,
Login screen awaits.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

Hi there! Thanks for sharing this work-in-progress. Implementing OAuth with Looker as a provider for private embed is an interesting use case.

I've reviewed the changes and focused on correctness, security, and maintainability, keeping in mind the WIP nature. My main concerns revolve around the security of the OAuth state parameter and opportunities to improve type safety and reduce code duplication.

My suggestions also aim to align with common TypeScript best practices, such as strong typing and the DRY (Don't Repeat Yourself) principle, to enhance code quality.

Please see the detailed comments below. Addressing the high-severity security issue with the state parameter would be crucial before this moves beyond a very early demo stage.

Summary of Findings

Security - CSRF Vulnerability in OAuth Flow: The state parameter in the oauth_login function is hardcoded (file demo/demo_single_frame.ts, line 537). This creates a Cross-Site Request Forgery (CSRF) vulnerability. The state parameter should be a unique, unpredictable value generated for each authentication request and verified upon callback.
Maintainability - Hardcoded OAuth Configuration: Several OAuth parameters (base_url, client_id, scope) in oauth_login are hardcoded (file demo/demo_single_frame.ts, lines 529, 534, 536). For better flexibility and maintainability, consider making these configurable.
Type Safety - Use of any: Several new functions (array_to_hex, secure_random, sha256_hash in demo/demo_single_frame.ts) use the any type for parameters or internal variables. Specifying more precise types (e.g., string, number, Uint8Array) would improve type safety, code clarity, and maintainability.
Code Duplication - Hex Conversion Logic: The logic for converting a byte array to a hex string in sha256_hash (file demo/demo_single_frame.ts, lines 560-561) is duplicated from the array_to_hex function. sha256_hash should reuse array_to_hex to adhere to the DRY (Don't Repeat Yourself) principle.

Merge Readiness

Given that this is a work-in-progress and there's a high-severity security concern (hardcoded OAuth state parameter), I recommend addressing this and other medium-severity suggestions before considering this for any further integration or wider use. As a reviewer, I am not authorized to approve pull requests, but I advise that these changes be made and re-reviewed. This PR should not be merged in its current state due to the identified security vulnerability.

gemini-code-assist · 2025-05-22T23:19:02Z

demo/demo_single_frame.ts

+    client_id: 'embed-server',
+    redirect_uri: `${location.origin}`,
+    scope: 'cors_api',
+    state: '1235813',


The state parameter is currently hardcoded. For OAuth 2.0 flows, the state parameter is crucial for preventing Cross-Site Request Forgery (CSRF) attacks. It must be a unique, unpredictable value that is generated by the client before initiating the authentication request and then validated when the authorization server redirects the user back.

Could you consider implementing a mechanism to:

Generate a cryptographically random string for state before redirecting.

Store this state value temporarily (e.g., in sessionStorage or a short-lived cookie).

When the user is redirected back to redirect_uri, retrieve the stored state and compare it with the state parameter received from the authorization server. If they don't match, the request should be rejected.

Using a hardcoded state significantly undermines the security of the OAuth flow. For more details, you can refer to RFC 6749, Section 10.12.

As this is a WIP, the suggestion below is a minimal marker. A full implementation would involve more logic.

Suggested change

state: '1235813',

state: '1235813', // TODO: Replace with a dynamically generated, unpredictable value for CSRF protection.

gemini-code-assist · 2025-05-22T23:19:03Z

demo/demo_single_frame.ts

+  const base_url = 'https://self-signed.looker.com:9999/auth'
+  const code_verifier = secure_random(32)
+  const code_challenge = await sha256_hash(code_verifier)
+  const params = {
+    response_type: 'code',
+    client_id: 'embed-server',
+    redirect_uri: `${location.origin}`,
+    scope: 'cors_api',


The base_url (line 529), client_id (line 534), and scope (line 536) are hardcoded here.

While this might be acceptable for an initial WIP demo, would it be beneficial to make these configurable? For instance, they could be sourced from demo_config.ts or environment variables, similar to how other configurations are handled in demo_config.ts. This would make the demo more adaptable and easier to configure for different environments or use cases.

gemini-code-assist · 2025-05-22T23:19:03Z

demo/demo_single_frame.ts

+function array_to_hex(array: any) {
+  return Array.from(array)
+    .map((b: any) => b.toString(16).padStart(2, '0'))
+    .join('')


The array parameter and the mapped item b are currently typed as any. Using more specific types would improve type safety and code readability.

Considering its usage with crypto.getRandomValues (which produces a Uint8Array), could array be typed as Uint8Array and b as number?

Suggested change

function array_to_hex(array: any) {

return Array.from(array)

.map((b: any) => b.toString(16).padStart(2, '0'))

.join('')

function array_to_hex(array: Uint8Array): string { // Type 'array' more specifically, e.g., Uint8Array

return Array.from(array)

.map((b: number) => b.toString(16).padStart(2, '0')) // Type 'b' as number

.join('');

gemini-code-assist · 2025-05-22T23:19:03Z

demo/demo_single_frame.ts

+    .join('')
+}
+
+function secure_random(byte_count: any) {


The byte_count parameter is typed as any. Would it be more appropriate to type it as number for better type safety?

Suggested change

function secure_random(byte_count: any) {

function secure_random(byte_count: number): string {

gemini-code-assist · 2025-05-22T23:19:03Z

demo/demo_single_frame.ts

+  return array_to_hex(array)
+}
+
+async function sha256_hash(message: any) {


The message parameter is typed as any. To improve type safety, could this be typed as string? Also, explicitly typing the return promise as Promise<string> would enhance clarity.

Suggested change

async function sha256_hash(message: any) {

async function sha256_hash(message: string): Promise<string> {

gemini-code-assist · 2025-05-22T23:19:03Z

demo/demo_single_frame.ts

+  const hashArray = Array.from(new Uint8Array(hashBuffer))
+  const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('')
+  return hashHex


The logic here to convert the hash buffer to a hex string seems to duplicate the functionality of the array_to_hex function.

Could we reuse array_to_hex here to avoid redundancy and adhere to the DRY (Don't Repeat Yourself) principle? This would also make the code more concise.

Suggested change

const hashArray = Array.from(new Uint8Array(hashBuffer))

const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('')

return hashHex

// const hashArray = Array.from(new Uint8Array(hashBuffer)); // This line can be removed

// const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join(''); // This line can be removed

return array_to_hex(new Uint8Array(hashBuffer)); // Reuse array_to_hex for conciseness and to apply DRY principle

wip: demo that uses Looker as an OAUTH provider for private embed

f19f371

This is very much a work in progress. Don't know when I will be able to finish it.

gemini-code-assist bot reviewed May 22, 2025

View reviewed changes

gemini-code-assist bot suggested changes May 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

wip: demo that uses Looker as an OAUTH provider for private embed #218

wip: demo that uses Looker as an OAUTH provider for private embed #218

Uh oh!

bryans99 commented May 22, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot May 22, 2025

Uh oh!

gemini-code-assist bot May 22, 2025

Uh oh!

gemini-code-assist bot May 22, 2025

Uh oh!

gemini-code-assist bot May 22, 2025

Uh oh!

gemini-code-assist bot May 22, 2025

Uh oh!

gemini-code-assist bot May 22, 2025

Uh oh!

Uh oh!

	state: '1235813',
	state: '1235813', // TODO: Replace with a dynamically generated, unpredictable value for CSRF protection.

	function secure_random(byte_count: any) {
	function secure_random(byte_count: number): string {

	async function sha256_hash(message: any) {
	async function sha256_hash(message: string): Promise<string> {

wip: demo that uses Looker as an OAUTH provider for private embed #218

Are you sure you want to change the base?

wip: demo that uses Looker as an OAUTH provider for private embed #218

Uh oh!

Conversation

bryans99 commented May 22, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

gemini-code-assist bot May 22, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot May 22, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot May 22, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot May 22, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot May 22, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!