Skip to content

[19.0][IMP] webservice, webservice_server_env: neutralize webservice backend credentials - #145

Open
yankinmax wants to merge 2 commits into
OCA:19.0from
camptocamp:imp-webservice-add-neutralize
Open

[19.0][IMP] webservice, webservice_server_env: neutralize webservice backend credentials#145
yankinmax wants to merge 2 commits into
OCA:19.0from
camptocamp:imp-webservice-add-neutralize

Conversation

@yankinmax

@yankinmax yankinmax commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@OCA-git-bot

OCA-git-bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Hi @etobella,
some modules you are maintaining are being modified, check this out!

@yankinmax
yankinmax force-pushed the imp-webservice-add-neutralize branch 2 times, most recently from ea302cf to 6502847 Compare July 9, 2026 15:15
@OCA-git-bot OCA-git-bot added the mod:webservice_server_env Module webservice_server_env label Jul 9, 2026
@yankinmax
yankinmax force-pushed the imp-webservice-add-neutralize branch from 14de1c4 to a29b302 Compare July 10, 2026 05:39
@OCA-git-bot OCA-git-bot added mod:webservice_server_env Module webservice_server_env and removed mod:webservice_server_env Module webservice_server_env labels Jul 10, 2026
@yankinmax yankinmax changed the title [19.0][IMP] webservice: neutralize webservice backend credentials [19.0][IMP] webservice, webservice_server_env: neutralize webservice backend credentials Jul 10, 2026
Ricardoalso
Ricardoalso reviewed Jul 10, 2026
View reviewed changes

@Ricardoalso Ricardoalso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks @yankinmax 🙏

Comments related to an open PR that will conflict

Comment thread webservice/data/neutralize.sql
Comment thread webservice_server_env/data/neutralize.sql
@yankinmax
yankinmax force-pushed the imp-webservice-add-neutralize branch 2 times, most recently from 0444013 to 87b843a Compare July 13, 2026 10:24
SilvioC2C
SilvioC2C reviewed Jul 16, 2026
View reviewed changes

@SilvioC2C SilvioC2C left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Needs updating on top of #143

Comment thread webservice/data/neutralize.sql
@yankinmax
[19.0][IMP] webservice: neutralize webservice backend credentials
4d16239
@yankinmax
[19.0][IMP] webservice_server_env: neutralize webservice backend cred…
8da6168 
…entials

If both webservice and server_environment are installed we also need neutralization for webservice_server_env
so the plain columns nulled by webservice/data/neutralize.sql can be properly cleared
@yankinmax
yankinmax force-pushed the imp-webservice-add-neutralize branch from 87b843a to 8da6168 Compare August 5, 2026 12:21
ivantodorovich
ivantodorovich approved these changes Aug 5, 2026
View reviewed changes

@ivantodorovich ivantodorovich left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks!

@ivantodorovich

ivantodorovich commented Aug 5, 2026

Copy link
Copy Markdown

@SilvioC2C @Ricardoalso this should be ok now, can you update your reviews? 🙏🏻

@OCA-git-bot

OCA-git-bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@ivantodorovich

ivantodorovich commented Aug 10, 2026

Copy link
Copy Markdown

@etobella / @simahawk can we merge this one? 🙏🏻

@SilvioC2C

SilvioC2C commented Aug 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@ivantodorovich hello, sorry but I was on vacation.
From what I can see, some of the new fields introduced by #143 are not neutralized by the .sql files' scripts. Is this wanted?

@ivantodorovich

ivantodorovich commented Aug 24, 2026

Copy link
Copy Markdown

@ivantodorovich hello, sorry but I was on vacation. From what I can see, some of the new fields introduced by #143 are not neutralized by the .sql files' scripts. Is this wanted?

The missing fields are:

  • oauth2_client_auth_method
  • oauth2_client_auth_header
  • oauth2_token_method

These don't hold any secrets, so IMO it's ok.
The only added field that would store a secret is: oauth2_client_auth_value, and that's on the list

SilvioC2C
SilvioC2C approved these changes Aug 25, 2026
View reviewed changes

@SilvioC2C SilvioC2C left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ivantodorovich thanks for the explanation, full green light from me then 🟢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
@ivantodorovich ivantodorovich ivantodorovich approved these changes
@SilvioC2C SilvioC2C SilvioC2C approved these changes
@Ricardoalso Ricardoalso Ricardoalso approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved mod:webservice_server_env Module webservice_server_env mod:webservice Module webservice ready to merge series:19.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yankinmax @OCA-git-bot @ivantodorovich @SilvioC2C @Ricardoalso