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

feat: Add enrollment setup and run tests with APKAM authentication #1321

Merged
merged 29 commits into from
Jun 14, 2024

Conversation

sitaram-kalluri
Copy link
Member

@sitaram-kalluri sitaram-kalluri commented May 20, 2024

- What I did

  • Modify the end-to-end tests to run with APKAM authentication.
  • Add "revoke" method to enrollment_service.dart to support "enroll:revoke" operation.
  • In AtClientImpl.dart, if the AtKey.namespace is null, we set the namespace from AtClientPreferences to the key. In collection framework, the namespace of the key is added to the "AtKey.key" and "AtKey.namespace" is unset. As a result the namespace from the "AtClientPreferences" is added causing duplicate/multiple namespaces being added to the key. Modified the code in "default_key_maker.dart" to fix the issue. Removed the namespace from the AtKey.key and added to AtKey.namespace.

- How I did it

  • Add "enrollment_setup.dart" script that submits an enrollment request to the server and approves the enrollment request.
    Upon approval of the request, the authentication keys are written to the atKeys file which are used by the tests for authentication.
  • The enrollment_setup.dart file is invoked by the github actions to generate at the atkeys file prior to running of tests.
  • In Config14.yaml, introduce "authType" variables which is set to "apkam" to run authenticate at the atsigns via the APKAM authentication. The tests run between the trunk version of sever - @ce2e1 and canary version of server -@ce2e4. In Config12.yaml, the "authType" is set to "pkam" to retain the existing way of running the tests.
  • In the "setUpAll" of each test, fetch the "authType" from the config.yaml to perform the authentication.
  • Since "notify:text" is being deprecated, removed the tests related to it.
  • In config14.yaml, the firstAtSign and secondAtSign are configured as "@ce2e1" and "@ce2e2" respectively, causing the tests to run against the trunk branch on both servers. However, the intended behavior of config14.yaml is to test against both the trunk branch and the canary version on the server. Therefore, updated the secondAtSign to "@ce2e4" and the fourthAtSign to "@ce2e2" to align with this intention.
  • Add "enrollment_teardown.dart" file which will revoke/deny the approved/pending enrollments respectively after the running the tests.

- How to verify it

  • All tests should pass.

- Description for the changelog

  • Run E2E tests with APKAM authentication.

@sitaram-kalluri sitaram-kalluri marked this pull request as ready for review May 27, 2024 07:37
@sitaram-kalluri sitaram-kalluri requested a review from gkc May 27, 2024 07:37
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

Excellent work @sitaram-kalluri. Just one change I see as necessary right now, see review comment

appName: 'wavi-$random',
deviceName: 'iphone',
otp: 'ABC123',
namespaces: {'wavi': 'rw', 'buzz': 'rw'});
Copy link
Contributor

Choose a reason for hiding this comment

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

We should clean up enrollments from previous test runs. In apkam_setup.sh the NoPorts e2e tests we do the following before submitting the new enrollment request for this test run:

  logInfo "Denying any pending enrollment requests for $atSign with apkamAppName $apkamApp and apkamDeviceName $apkamDev"
  $authBinary deny -r "$atDirectoryHost" -a "$atSign" --arx "$apkamApp" --drx "$apkamDev" || return $?

  logInfo "Revoking any approved enrollment requests for $atSign with apkamAppName $apkamApp and apkamDeviceName $apkamDev"
  $authBinary revoke -r "$atDirectoryHost" -a "$atSign" --arx "$apkamApp" --drx "$apkamDev" || return $?

where $apkamApp has been set to "e2e_all"

For these tests I suggest

  • use an apkam appName of 'e2e_test'
  • before submitting the new enrollment request:
    • fetch all pending enrollment requests where the appName is 'e2e_test', and deny all of them
    • fetch all approved enrollment requests where the appName is 'e2e_test', and revoke all of them

@@ -16,17 +19,32 @@ class TestSuiteInitializer {
return _singleton;
}

Future<void> testInitializer(String atSign, String namespace) async {
Future<void> testInitializer(String atSign, String namespace,
{String authType = 'pkam'}) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

For clarity in the test cases which use this function, would it be better to make authType a required parameter so that it's always explicit in the test code which authType is being used?

@sitaram-kalluri sitaram-kalluri linked an issue Jun 10, 2024 that may be closed by this pull request
@sitaram-kalluri
Copy link
Member Author

Currently, the E2E tests are running successfully. The unit tests are failing with dart beta version due to following dart issue: dart-lang/sdk#55911 (comment). Pending work is to update the dependencies and check if the issue is resolved.

@sitaram-kalluri sitaram-kalluri requested a review from gkc June 14, 2024 10:06
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

Great work. Thank you @sitaram-kalluri

@gkc gkc merged commit c9f2485 into trunk Jun 14, 2024
11 checks passed
@gkc gkc deleted the 1309-need-some-e2e-tests-for-apkam-enrolled-clients branch June 14, 2024 10:46
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.

Need some e2e tests for APKAM-enrolled clients
2 participants