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

[IMP] sign_oca: Allow to store geolocalization #16

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

etobella
Copy link
Member

Using geolocalization can be useful in order to prove the authorship of the signature

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale label Aug 25, 2024
@marcelsavegnago
Copy link
Sponsor Member

@etobella can you rebase please ?

@marcelsavegnago
Copy link
Sponsor Member

@WesleyOliveira98, could you please review?

Copy link

@WesleyOliveira98 WesleyOliveira98 left a comment

Choose a reason for hiding this comment

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

[FUNCTIONAL REVIEW]

Create a Sign Oca Template with Ask Location enabled
image

Testing on Runboat (which uses HTTP), the browser blocks geolocation retrieval. This is a standard security restriction for pages not served over HTTPS. As a result, it consistently returns an error.

TypeError: Cannot read properties of undefined (reading 'latitude')
    at SignOcaPdfPortal._onClickSign (http://oca-sign-16-0-pr16-b490badf47d5.runboat.odoo-community.org/web/assets/212-b5b3804/web.assets_frontend.min.js:9355:80)

To work around this, I had to run Chrome with the following command (using Ubuntu):

google-chrome --unsafely-treat-insecure-origin-as-secure="http://oca-sign-16-0-pr16-b490badf47d5.runboat.odoo-community.org/" --user-data-dir=/tmp/chrome_dev_test

This allowed me to enable location access for Runboat and proceed with the signing process. It worked perfectly, and the system correctly retrieved my latitude and longitude.
image

And then, i reopened Chrome with the flags and blocked the site from accessing the geolocation, which resulted in the same error as before.

Comment on lines +69 to +72
kwargs: {
latitude: position.coords.latitude,
longitude: position.coords.longitude,
},

Choose a reason for hiding this comment

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

I think you should prevent the error in the code by handling the possibility of geolocation not being available. For example:

latitude: position ? position.coords.latitude : false,
longitude: position ? position.coords.longitude : false,

Comment on lines +52 to +56
params: {
items: this.info.items,
latitude: position.coords.latitude,
longitude: position.coords.longitude,
},

Choose a reason for hiding this comment

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

I think you should prevent the error in here as well

@@ -430,7 +434,7 @@ def sign(self):
"url": self.access_url,
}

def action_sign(self, items, access_token=False):
def action_sign(self, items, access_token=False, latitude=False, longitude=False):

Choose a reason for hiding this comment

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

Additionally, in the actions_sign method, you could add a ValidationError if the template is marked as ask_location and the latitude and longitude are not provided.

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.

4 participants