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

IDEMPIERE-5169 OAuth2: add same email account on other client will break it on old client #1146

Merged

Conversation

CarlosRuiz-globalqss
Copy link
Collaborator

https://idempiere.atlassian.net/browse/IDEMPIERE-5169

Opening this in draft because is not tested against real potential cases:

  • column AD_AuthorizationAccount.AccessToken encrypted and unencrypted
  • multitenant
  • potentially different encryption keys in different tenants

Copy link
Contributor

@hieplq hieplq left a comment

Choose a reason for hiding this comment

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

after fix review comment then i test success but not-success with encrypt case by issue report at IDEMPIERE-5168

  1. i get bellow exception when run encrypt process because i have accessKey when encrypt make length > 8000 character
    23:59:37.207===========> ColumnEncryption.changeFieldLength: EncryptError [ChangeFieldLength]: ColumnID=214402, NewLength=8016 [274] 23:59:37.218===========> ColumnEncryption.process: java.lang.Exception [274] java.lang.Exception at org.compiere.process.ColumnEncryption.changeFieldLength(ColumnEncryption.java:519)

  2. i change data type of accessKey to textLong by that i can't run encrypt process by readonly logic

@CarlosRuiz-globalqss
Copy link
Collaborator Author

2. i change data type of accessKey to textLong by that i can't run encrypt process by readonly logic

I haven't tested, if CLOB columns can be encrypted by iDempiere, we can add that one to the readonly logic.

@hieplq
Copy link
Contributor

hieplq commented Jan 30, 2022

if CLOB columns can be encrypted by iDempiere

i saw binary and text long on list readonly. by that logic i think image type also need to add to list
but how to resolve IDEMPIERE-5168?
readOnly

@CarlosRuiz-globalqss
Copy link
Collaborator Author

if CLOB columns can be encrypted by iDempiere

i saw binary and text long on list readonly. by that logic i think image type also need to add to list but how to resolve IDEMPIERE-5168? readOnly

I mean, if you can test removing the "Text" reference ID from the read-only logic and test if iDempiere is capable of encrypting/decrypting correctly a CLOB value, I don't see clearly a reason why iDempiere would not be able to do it

@hieplq hieplq marked this pull request as ready for review January 31, 2022 14:14
Copy link
Contributor

@hieplq hieplq left a comment

Choose a reason for hiding this comment

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

test passed for whole case with support of IDEMPIERE-5168

@CarlosRuiz-globalqss CarlosRuiz-globalqss merged commit 824e1bb into idempiere:master Feb 4, 2022
@CarlosRuiz-globalqss CarlosRuiz-globalqss deleted the IDEMPIERE-5169 branch February 4, 2022 10:28
CarlosRuiz-globalqss added a commit that referenced this pull request Feb 5, 2022
…eak it on old client (#1146)

* IDEMPIERE-5169 OAuth2: add same email account on other client will break it on old client

* Fix wrong SQL - thanks to HiepLq

* Fix wrong parameters - thanks to HiepLq
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants