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

Allow Google ReCaptcha Enterprise #239

Merged
merged 14 commits into from
May 29, 2023
Merged

Allow Google ReCaptcha Enterprise #239

merged 14 commits into from
May 29, 2023

Conversation

kuhnsm
Copy link
Contributor

@kuhnsm kuhnsm commented Jan 5, 2022

This change allows Google ReCaptcha Enterprise usage. You can set the windows global dynamic option, enterprise to true to get this functionality.

I have an Enterprise account and I tested there.

Please let me know if you have any questions/comments.
Thanks.

@9kartik
Copy link

9kartik commented Jan 28, 2022

In general, I was also wondering can we not just simply replace the this.grecaptcha method with this.grecaptcha.enterprise in the render method (if it exists)? I'm yet to try that out, but should mostly work.

@pinksynth
Copy link

If this works, can we get it merged in? My team is using a fork for enterprise, but it'd be great to see it adopted into this lib instead. Let me know if there's anything I could do to help.

Thanks!

Copy link

@moonkid196 moonkid196 left a comment

Choose a reason for hiding this comment

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

I'd make sure to double-check the modules diligently, and perhaps remove the hostname interpolation when generating the URL

@@ -11,6 +11,9 @@ function getOptions() {
function getURL() {
const dynamicOptions = getOptions();
const hostname = dynamicOptions.useRecaptchaNet ? "recaptcha.net" : "www.google.com";
if (dynamicOptions.enterprise) {
return `https://${hostname}/recaptcha/enterprise.js?onload=${callbackName}&render=explicit`;

Choose a reason for hiding this comment

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

is recaptcha.net a valid domain for enterprise? I've never seen this myself. Might be safer to assume not unless you can confirm

Copy link
Owner

Choose a reason for hiding this comment

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

@moonkid196 I would think both options would work because recaptcha.net is really just a redirect on the other domain to avoid blocks on google's domain

Comment on lines +14 to 20
let getResponse = this.props.grecaptcha && this.props.grecaptcha.getResponse;
if (this.props.grecaptcha.enterprise) {
getResponse = this.props.grecaptcha.enterprise.getResponse();
}
if (getResponse && this._widgetId !== undefined) {
return getResponse(this._widgetId);
}

Choose a reason for hiding this comment

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

IIUC, you're trying to figure out the method to call? I feel like this is more complex than it needs to be. The biggest condition is that the widget is undefined, so maybe rule that out first?

Suggested change
let getResponse = this.props.grecaptcha && this.props.grecaptcha.getResponse;
if (this.props.grecaptcha.enterprise) {
getResponse = this.props.grecaptcha.enterprise.getResponse();
}
if (getResponse && this._widgetId !== undefined) {
return getResponse(this._widgetId);
}
if (this._widgetId === undefined || this.props.grecaptcha === undefined) {
return null;
}
if (this.props.grecaptcha.enterprise) {
return this.props.grecaptcha.enterprise.getResponse(this._widgetId);
}
return this.props.grecaptcha.getResponse(this._widgetId);
}

Either way, you may want to remove the parentheses on line 16, I think

Comment on lines +53 to +56
let resetter = this.props.grecaptcha.reset;
if (this.props.grecaptcha.enterprise.reset) {
resetter = this.props.grecaptcha.enterprise.reset;
}

Choose a reason for hiding this comment

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

Think you need to check the intervening values here too:

Suggested change
let resetter = this.props.grecaptcha.reset;
if (this.props.grecaptcha.enterprise.reset) {
resetter = this.props.grecaptcha.enterprise.reset;
}
if (this._widgetId === undefined || this.props.grecaptcha === undefined) {
this._executeRequested = true;
else if (this.props.grecaptcha.enterprise) {
this.props.grecaptcha.enterprise.reset(this._widgetId);
else {
this.props.grecaptcha.reset(this._widgetId);
}

Comment on lines +102 to +105
this.props.grecaptcha.enterprise &&
this.props.grecaptcha.enterprise.render
) {
render = this.props.grecaptcha.enterprise.render;

Choose a reason for hiding this comment

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

probably don't need this last one:

Suggested change
this.props.grecaptcha.enterprise &&
this.props.grecaptcha.enterprise.render
) {
render = this.props.grecaptcha.enterprise.render;
this.props.grecaptcha.enterprise
) {
render = this.props.grecaptcha.enterprise.render;

... although the original code has this shrug

@andrralv
Copy link

andrralv commented Nov 9, 2022

Hi, is there a workaround to make this library work with enterprise without this PR being merged as of now?

@DATeclipse
Copy link

When is this change expected to be merged?

@moonkid196
Copy link

Hi, is there a workaround to make this library work with enterprise without this PR being merged as of now?

I borrowed some ideas from this PR and implemented it myself with the following notes, steps:

First, I use the flow from create-react-app; ymmv

Next, added a line like this to the anchoring/static index.html body:

+ <script src="https://www.google.com/recaptcha/enterprise.js?render=XXXXXXXXXXXX" async defer></script>

Created a stripped-down version of this PR, with my own elements (I use firebase, so I have a function to call assessment API referenced here and return the result):

class Recaptcha {
  constructor() {
    this.recaptcha = window.grecaptcha;
    this.checkLoaded = this.checkLoaded.bind(this);
    this.isLoaded = this.isLoaded.bind(this);
    this.justVerify = this.justVerify.bind(this);
    this.timerID = setInterval(this.checkLoaded, 100);
  }
  async checkLoaded() {
    if (!this.recaptcha && window.grecaptcha) {
      this.recaptcha = window.grecaptcha;
      console.info("recaptcha library loaded");
    }
  }
  isLoaded() {
    return (this.recaptcha !== undefined);
  }
  async getToken(action) {
    if (!this.isLoaded()) {
      throw new Error("recaptcha libraries not yet loaded");
    }
    const token = await this.recaptcha.enterprise.execute(
      RecaptchaSiteKey, { action: action }
    );
    return token;
  }
  async justVerify(action) {
    if (!this.isLoaded()) {
      throw new Error("recaptcha libraries not yet loaded");
    }
    this.recaptcha.enterprise.ready(async () => {
      const token = await this.recaptcha.enterprise.execute(
        RecaptchaSiteKey, { action: action }
      );
      const result = await justVerify(
        { token: token, action: action }
      );
      console.debug(
        `recaptcha result: ${action}: ${JSON.stringify(result)}`
      );
    });
  }
}

Import it and pass it into routes (using react router here):

+const recaptcha = new Recaptcha();
 const root = ReactDOM.createRoot(document.getElementById('root'));
 root.render(
   <React.StrictMode>
     <BrowserRouter>
       <Routes>
-        <Route path="/" element={<App />} />
+        <Route path="/" element={<App recaptcha={recaptcha} />} />

Set up some state in App:

+   this.state = { loaded: true };
+   this.pageUtils = new PageUtils(this);

And finally, the definition of page utils that handles the loading, kicks off the assessment:

class PageUtils {
  constructor(component) {
    this.component = component;
    this.firstLoad = true;
    this.ensureLoaded = this.ensureLoaded.bind(this);
    this.navClick = this.navClick.bind(this);
    this.timerID = setInterval(this.ensureLoaded, 100);

    if (!this.component.props.recaptcha.isLoaded()) {
      this.component.state.loaded = false;
    }
  }
  async ensureLoaded() {
    if (
      !this.component.state.loaded && this.component.props.recaptcha.isLoaded()
    ) {
      this.component.setState({ loaded: true });
    }

    if (this.component.state.loaded && this.firstLoad) {
      await this.component.props.recaptcha.justVerify(RecaptchaActions.PageLoad);
      if (this.timerID) {
        clearInterval(this.timerID);
      }
    }
  }
  async navClick(event) {
    await this.component.props.recaptcha.justVerify(RecaptchaActions.UserClick);
  }
}

It's not the best code, and when I have time, I'll probably improve it in a few ways (use the lifecycle methods to trigger the page-load assessments, maybe turn PageUtils into a (hidden) component, for example), but it may help get you going

@zachmerrill
Copy link

What's holding back this PR from being merged?

@nike1v
Copy link

nike1v commented May 23, 2023

@dozoisch Can you please merge this, if it has no conflicts? Thanks!

@dozoisch dozoisch merged commit 6a2ca83 into dozoisch:master May 29, 2023
@dozoisch
Copy link
Owner

@nike1v it's been merged, but had some small issues. If you can take a look at #270

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.

9 participants