-
Notifications
You must be signed in to change notification settings - Fork 2
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
ROKMETRO Updates #707
base: develop
Are you sure you want to change the base?
ROKMETRO Updates #707
Conversation
* add app-env.json and update port * Update app-env.json * Update app-env.json * update detect-secrets, update secrets baseline * update secrets baseline * update makefile versioning * Update Makefile Fix typo * print version * upgrade go to v1.20 * fix secrets * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * [rokwire#659] WebAuthn authentication (#7) * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * fix error handling * fix login issues for mobile * upgrade dependencies * [rokwire#659] webauthn authentication (#8) * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * fix error handling * fix login issues for mobile * upgrade dependencies * add webauthn to account check types * add configs for authenticator selection to supported auth type params * add configs for authenticator selection to supported auth type params (#10) * start adding verification types (contains errors) [rokwire#665] * continue splitting auth and verification types [rokwire#665] * finish implementing password auth type, start code verification type, add phone verifier interface [rokwire#665] * finish refactoring identifier, auth types, start updating apis [rokwire#665] * upgrade dependencies * finish fixing errors [rokwire#665] * fix passkey errors [rokwire#665] * bug fixes, email with passkey not working because no params in email auth type * update identifier impl and auth impl getters to better handle backwards compatibility (has errors) * bug fixes, email and passkey not completing registration * add json omitempty tags to credential structs * better identifier type parsing * passkeys using email and username identifiers working * start fixing phone, passkey auth * bug fixes for phone and passkey, better error messages * simplify phone verifier interface * phone auth type link working, add authCommunicationChannel interface to handle verification functions * add ability to link webauthn credentials to accounts * only set username if empty * Change messages handling for verification * remove commented blocks * cleanup * return verified auth types when cannot find account with username but not identifier * bug fixes * fix phone auth type docs * make sure usernames are lowercase, do not fail if phone verifier fails to init * start refactoring account auth types into account auth types and account identifiers, do not store identifiers in credentials (contains errors) * more progress, identifier type and auth type simplifications * read email verification settings from config, refactor verify credential APIs to verify identifier * build error fixes, add account identifier storage operations * start preparing for multiple credentials of same type (e.g., passkeys) * some progress implementing passkey with identifier flow * rewrite passkey flows * build error cleanup, start moving account external IDs into identifiers, refactor shared profile stuff * start fixing shared profiles * comment shared profile functionality * update API docs for linking auth types and identifiers, more error fixes * more API doc tweaks, more error fixes, remove all uses of claims.UID * more link and unlink request body tweaks * do not store account auth type ID in login session, instead use identifier to get account * start working on link account auth type implementation updates * more link auth type updates * finish implementing link identifier, some code auth type bug fixes * match LinkAccountIdentifier interface definition * fix remaining build errors, begin implementing DB migration * add support for login using external identifiers, update more request body definitions * identifiers bug fixes, fix build errors * update db indexes * start implementing db migration * do not allow generic oidc auth type code - no specified identity provider * implement app org and auth type migrations * credentials migration working, use json convert utils func * login session migration done * accounts migration done, a few bug fixes related to external IDs * move migration functions into separate file * more bug fixes, accountAuthTypesToDef not working * email and phone login fixes, finish implementing identifier-less login * bug fixes * linking, unlinking bug fixes * more linking, unlinking bug fixes and identifier verification email bug fixes * username login, webauthn backwards compatible login bug fixes * fix identifier-less webauthn login, update canLink * fix passkey sign up * return account on webauthn signup * make OIDC ID tokens optional * add sign-in-options API, update login API to accept account identifier ID * finish implementing sign in options and login with identifier ID * mask email and phone identifiers for sign in options, add regexp to validate emails * clean up linking, unlinking * fix unlink examples, sign in options fix, handle nil identifier when linking * upgrade dependencies, set username in token claims * use error statuses for auth type and identifier linking * allow webauthn credentials to be created after account already exists * update changelog * updates and fixes for conde_oidc, started refactoring email and phone from profile, username from account into identifiers * fix build errors * start handling external email identifiers * implement profile email and phone and account username migrations * set sensitive flags for email and phone migrations * finish implementing identifier sensitive field, return profile email, phone and username for BC * disallow updating account username to empty string * add sensitive field to account identifier api model * bug fixes * mark email as external if it matches external email field * usernames verified by default, identifiers used to sign up with webauthn unverified by default * auth type unlink bug fixes * simplify link auth type transaction, add app type identifier to webauthn aat params * fix link docs example * return identifiers on auth type link and unlink * improve external identifier migration, add IsEmailVerified flag * do not change email sensitivity on update external identifiers * remove API docs comment * fix go mod --------- Co-authored-by: Stephen Hurwit <[email protected]> Co-authored-by: Stephen Hurwit <[email protected]> Co-authored-by: akshadpai <[email protected]>
* add app-env.json and update port * Update app-env.json * Update app-env.json * update detect-secrets, update secrets baseline * update secrets baseline * update makefile versioning * Update Makefile Fix typo * print version * upgrade go to v1.20 * fix secrets * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * [rokwire#659] WebAuthn authentication (#7) * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * fix error handling * fix login issues for mobile * upgrade dependencies * [rokwire#659] webauthn authentication (#8) * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * fix error handling * fix login issues for mobile * upgrade dependencies * add webauthn to account check types * add configs for authenticator selection to supported auth type params * add configs for authenticator selection to supported auth type params (#10) * start adding verification types (contains errors) [rokwire#665] * continue splitting auth and verification types [rokwire#665] * finish implementing password auth type, start code verification type, add phone verifier interface [rokwire#665] * finish refactoring identifier, auth types, start updating apis [rokwire#665] * upgrade dependencies * finish fixing errors [rokwire#665] * fix passkey errors [rokwire#665] * bug fixes, email with passkey not working because no params in email auth type * update identifier impl and auth impl getters to better handle backwards compatibility (has errors) * bug fixes, email and passkey not completing registration * add json omitempty tags to credential structs * better identifier type parsing * passkeys using email and username identifiers working * start fixing phone, passkey auth * bug fixes for phone and passkey, better error messages * simplify phone verifier interface * phone auth type link working, add authCommunicationChannel interface to handle verification functions * add ability to link webauthn credentials to accounts * only set username if empty * Change messages handling for verification * remove commented blocks * cleanup * return verified auth types when cannot find account with username but not identifier * bug fixes * fix phone auth type docs * make sure usernames are lowercase, do not fail if phone verifier fails to init * start refactoring account auth types into account auth types and account identifiers, do not store identifiers in credentials (contains errors) * more progress, identifier type and auth type simplifications * read email verification settings from config, refactor verify credential APIs to verify identifier * build error fixes, add account identifier storage operations * start preparing for multiple credentials of same type (e.g., passkeys) * some progress implementing passkey with identifier flow * rewrite passkey flows * build error cleanup, start moving account external IDs into identifiers, refactor shared profile stuff * start fixing shared profiles * comment shared profile functionality * update API docs for linking auth types and identifiers, more error fixes * more API doc tweaks, more error fixes, remove all uses of claims.UID * more link and unlink request body tweaks * do not store account auth type ID in login session, instead use identifier to get account * start working on link account auth type implementation updates * more link auth type updates * finish implementing link identifier, some code auth type bug fixes * match LinkAccountIdentifier interface definition * fix remaining build errors, begin implementing DB migration * add support for login using external identifiers, update more request body definitions * identifiers bug fixes, fix build errors * update db indexes * start implementing db migration * do not allow generic oidc auth type code - no specified identity provider * implement app org and auth type migrations * credentials migration working, use json convert utils func * login session migration done * accounts migration done, a few bug fixes related to external IDs * move migration functions into separate file * more bug fixes, accountAuthTypesToDef not working * email and phone login fixes, finish implementing identifier-less login * bug fixes * linking, unlinking bug fixes * more linking, unlinking bug fixes and identifier verification email bug fixes * username login, webauthn backwards compatible login bug fixes * fix identifier-less webauthn login, update canLink * fix passkey sign up * return account on webauthn signup * make OIDC ID tokens optional * add sign-in-options API, update login API to accept account identifier ID * finish implementing sign in options and login with identifier ID * mask email and phone identifiers for sign in options, add regexp to validate emails * clean up linking, unlinking * fix unlink examples, sign in options fix, handle nil identifier when linking * upgrade dependencies, set username in token claims * use error statuses for auth type and identifier linking * allow webauthn credentials to be created after account already exists * update changelog * updates and fixes for conde_oidc, started refactoring email and phone from profile, username from account into identifiers * fix build errors * start handling external email identifiers * implement profile email and phone and account username migrations * set sensitive flags for email and phone migrations * finish implementing identifier sensitive field, return profile email, phone and username for BC * disallow updating account username to empty string * add sensitive field to account identifier api model * bug fixes * mark email as external if it matches external email field * usernames verified by default, identifiers used to sign up with webauthn unverified by default * auth type unlink bug fixes * simplify link auth type transaction, add app type identifier to webauthn aat params * fix link docs example * return identifiers on auth type link and unlink * improve external identifier migration, add IsEmailVerified flag * do not change email sensitivity on update external identifiers * remove API docs comment * fix go mod * fix webauthn beginLogin with identifier (missing user.Name) --------- Co-authored-by: Stephen Hurwit <[email protected]> Co-authored-by: Stephen Hurwit <[email protected]> Co-authored-by: akshadpai <[email protected]>
* start implementing account secrets - added to core and API models, added encryption, decryption utils [rokwire#677] * implement service AES key management [rokwire#677] * update mock storage [rokwire#677] * add update secrets API, add decrypt secrets func to auth interface [rokwire#677] * add missing files * implement secrets decryption on get account and login [rokwire#677] * bug fixes and cleanup, add timestamp fields to key model (RSA-PSS parsing not working) [rokwire#677] * update changelog
* add app-env.json and update port * Update app-env.json * Update app-env.json * update detect-secrets, update secrets baseline * update secrets baseline * update makefile versioning * Update Makefile Fix typo * print version * upgrade go to v1.20 * fix secrets * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * [rokwire#659] WebAuthn authentication (#7) * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * fix error handling * fix login issues for mobile * upgrade dependencies * [rokwire#659] webauthn authentication (#8) * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * fix error handling * fix login issues for mobile * upgrade dependencies * add webauthn to account check types * add configs for authenticator selection to supported auth type params * add configs for authenticator selection to supported auth type params (#10) * start adding verification types (contains errors) [rokwire#665] * continue splitting auth and verification types [rokwire#665] * finish implementing password auth type, start code verification type, add phone verifier interface [rokwire#665] * finish refactoring identifier, auth types, start updating apis [rokwire#665] * upgrade dependencies * finish fixing errors [rokwire#665] * fix passkey errors [rokwire#665] * bug fixes, email with passkey not working because no params in email auth type * update identifier impl and auth impl getters to better handle backwards compatibility (has errors) * bug fixes, email and passkey not completing registration * add json omitempty tags to credential structs * better identifier type parsing * passkeys using email and username identifiers working * start fixing phone, passkey auth * bug fixes for phone and passkey, better error messages * simplify phone verifier interface * phone auth type link working, add authCommunicationChannel interface to handle verification functions * add ability to link webauthn credentials to accounts * only set username if empty * Change messages handling for verification * remove commented blocks * cleanup * return verified auth types when cannot find account with username but not identifier * bug fixes * fix phone auth type docs * make sure usernames are lowercase, do not fail if phone verifier fails to init * start refactoring account auth types into account auth types and account identifiers, do not store identifiers in credentials (contains errors) * more progress, identifier type and auth type simplifications * read email verification settings from config, refactor verify credential APIs to verify identifier * build error fixes, add account identifier storage operations * start preparing for multiple credentials of same type (e.g., passkeys) * some progress implementing passkey with identifier flow * rewrite passkey flows * build error cleanup, start moving account external IDs into identifiers, refactor shared profile stuff * start fixing shared profiles * comment shared profile functionality * update API docs for linking auth types and identifiers, more error fixes * more API doc tweaks, more error fixes, remove all uses of claims.UID * more link and unlink request body tweaks * do not store account auth type ID in login session, instead use identifier to get account * start working on link account auth type implementation updates * more link auth type updates * finish implementing link identifier, some code auth type bug fixes * match LinkAccountIdentifier interface definition * fix remaining build errors, begin implementing DB migration * add support for login using external identifiers, update more request body definitions * identifiers bug fixes, fix build errors * update db indexes * start implementing db migration * do not allow generic oidc auth type code - no specified identity provider * implement app org and auth type migrations * credentials migration working, use json convert utils func * login session migration done * accounts migration done, a few bug fixes related to external IDs * move migration functions into separate file * more bug fixes, accountAuthTypesToDef not working * email and phone login fixes, finish implementing identifier-less login * bug fixes * linking, unlinking bug fixes * more linking, unlinking bug fixes and identifier verification email bug fixes * username login, webauthn backwards compatible login bug fixes * fix identifier-less webauthn login, update canLink * fix passkey sign up * return account on webauthn signup * make OIDC ID tokens optional * add sign-in-options API, update login API to accept account identifier ID * finish implementing sign in options and login with identifier ID * mask email and phone identifiers for sign in options, add regexp to validate emails * clean up linking, unlinking * fix unlink examples, sign in options fix, handle nil identifier when linking * upgrade dependencies, set username in token claims * use error statuses for auth type and identifier linking * allow webauthn credentials to be created after account already exists * update changelog * updates and fixes for conde_oidc, started refactoring email and phone from profile, username from account into identifiers * fix build errors * start handling external email identifiers * implement profile email and phone and account username migrations * set sensitive flags for email and phone migrations * finish implementing identifier sensitive field, return profile email, phone and username for BC * disallow updating account username to empty string * add sensitive field to account identifier api model * bug fixes * mark email as external if it matches external email field * usernames verified by default, identifiers used to sign up with webauthn unverified by default * auth type unlink bug fixes * simplify link auth type transaction, add app type identifier to webauthn aat params * fix link docs example * return identifiers on auth type link and unlink * improve external identifier migration, add IsEmailVerified flag * do not change email sensitivity on update external identifiers * remove API docs comment * fix go mod * fix webauthn beginLogin with identifier (missing user.Name) * move phone and email validation to utils, validate profile phone and email on account migration * delete login states once used successfully, fix email and code login * add context to UpdateCredentialValue storage function * merged changes from rokwire-674 --------- Co-authored-by: Stephen Hurwit <[email protected]> Co-authored-by: Stephen Hurwit <[email protected]> Co-authored-by: akshadpai <[email protected]>
…se identifiers in account search API examples
…during auth type migration
…equestor is not scoped for identifiers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @roberlander2 The review is in progress.
It is a huge PR and it will take time until I pass through it. I noticed two issues and marked them in the code lines. Thanks
@@ -23,9 +23,8 @@ type application struct { | |||
ID string `bson:"_id"` | |||
Name string `bson:"name"` | |||
|
|||
MultiTenant bool `bson:"multi_tenant"` | |||
Admin bool `bson:"admin"` | |||
Code string `bson:"code"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I know the code field is used in the client apps so that the client to give more approariate UI name of the application but not the one in the name field. This was used in the events UI. I am not familiar with the client code but if not removed then maybe we still need this field.
cc @mihail-varbanov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out @petyos. I looked through the client code for a bit and could not find where the Code
field is used, but please let me know if it is still being used and I will undo this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked with @mihail-varbanov too. It seems this code using is no more used indeed. Thanks.
driven/storage/migrations.go
Outdated
return nil | ||
} | ||
|
||
// migrate to tenants accounts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already removed all this code for tenants accounts migration as it was deployed on all environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these changes for the migration back in to perform the migration in the ROKMETRO database. I realize I made a mistake in changing the "migrated_2"
field name to "migrated"
, which would cause the migration to run again on accounts in the ROKWIRE database. I fixed this issue so that this migration that I re-introduced will not affect the already migrated ROKWIRE accounts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we remove all this migration code related with the tenants accounts? Or this is not applied into the Rokmetro databases yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the tenant account migration has not yet been applied to the Rokmetro databases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @roberlander2. I understand but I think we should merge into the Rokwire code only the new stuff you have developed and changed but not old Rokwire code. In the latest code in Rokwire the tenant accounts migration code does not exist. It was removed with this issue #698 We had issues with the tenants accounts migration and we reverted the version the first time. Finally we fixed it and all now in Rokwire is working as expected and the migration code was removed. Maybe you should get the latest code state from Rokwire develop and merge into your branch. As you have not deployed this yet then maybe better to put this tenant migration code alive but only in the Rokmetro world not introducing it again in Rokwire with your PR. In addition to your huge PR including couple issues handled, some refactoring and your migration stuff, keeping also the tenants migration code complicates the transfer of your code into the Rokwire code and poses a risk for issues. Let me know if you have any comments. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @petyos, I think you are right that we should not reintroduce the tenant accounts migration, so I removed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @roberlander2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @roberlander2 ,
I am back on reviewing this. The review is in progress. I left two comments. Also, do we need to add (or update) any environment variables with these changes? Thanks.
alg = keys.RS256 | ||
} | ||
authPrivKey, err := keys.NewPrivKey(alg, authPrivKeyPem) | ||
oldSupportLegacySigsStr := envLoader.GetAndLogEnvVar("ROKWIRE_CORE_OLD_SUPPORT_LEGACY_SIGNATURES", false, false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between the new and the old legacy signatures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is to account for the potential to change from a RS256
key to a PS256
key. The legacy signature support is used in parsePrivKeyFromEnvVar
to set the correct algorithm for both the old key and the new key.
Thank you @petyos, There are five new environment variables (listed below) and all are optional. The first two are used to set parameters that determine how verification emails are sent to users and the last three are used to support private key rotation for the service (we should only set them when we want to perform a rotation).
|
…ne to be updated in identifiers
…allow beginRegistration for webauthn in checkCredentials
Ok, thanks! |
@roberlander2 remember to update rokmetro:rokwire-develop with changes from rokmetro:develop in addition to rokwire:develop. |
Description
This PR merges in updates from the ROKMETRO team, which includes WebAuthn authentication, a split of existing
AccountAuthType
data intoAccountAuthType
andAccountIdentifier
to allow users to authenticate with any identifier/auth type combination, account following support, app org token duration policies, and various bug fixes and logging improvements.All of these changes should be backwards compatible with existing client applications and existing building blocks. This is a combination of many different changes, so please let me know if you notice any issues or have questions or concerns. Thanks!
Resolves #659, #677, #472, #674
Review Time Estimate
Please give your idea of how soon this pull request needs to be reviewed by selecting one of the options below. This can be based on the criticality of the issue at hand and/or other relevant factors.
Type of changes
Please select a relevant option:
Checklist:
Please select all applicable options: