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.

Sign up for GitHub

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

Jump to bottom

[tests-only][full-ci] adding test for deleting auth-app token using the token received from auth-app list request #10814

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

[tests-only][full-ci] adding test for deleting auth-app token using the token received from auth-app list request #10814

wants to merge 1 commit into from

Conversation

nirajacharya2
Copy link
Contributor

@nirajacharya2 nirajacharya2 commented Dec 31, 2024 •
edited by amrita-shrestha
Loading

Description

This PR adds test for deleting auth-app token using the token received from auth-app list request

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1: locally
  • test case 2: ci
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@nirajacharya2 nirajacharya2 self-assigned this Dec 31, 2024
@nirajacharya2 nirajacharya2 mentioned this pull request Dec 31, 2024
Open
22 tasks
@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch 2 times, most recently from c344afd to ee95a6e Compare January 10, 2025 08:55
@nirajacharya2 nirajacharya2 changed the title [tests-only][full-ci] adding test for deleting auth token using cli [tests-only][full-ci] adding test for deleting auth-app token Jan 10, 2025
@nirajacharya2 nirajacharya2 marked this pull request as ready for review January 10, 2025 08:57
@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch from ee95a6e to 6688f7d Compare January 13, 2025 03:31
@nirajacharya2 nirajacharya2 requested a review from saw-jan January 13, 2025 03:33
saw-jan
saw-jan reviewed Jan 13, 2025
View reviewed changes
tests/acceptance/bootstrap/AuthAppContext.php Outdated Show resolved Hide resolved
tests/acceptance/bootstrap/AuthAppContext.php Outdated Show resolved Hide resolved
tests/acceptance/bootstrap/AuthAppContext.php Outdated Show resolved Hide resolved
@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch from 6688f7d to bdb3337 Compare January 13, 2025 06:16
@nirajacharya2 nirajacharya2 requested a review from saw-jan January 13, 2025 06:16
@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch from ff5d546 to f07448a Compare January 13, 2025 11:06
amrita-shrestha
amrita-shrestha requested changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@amrita-shrestha amrita-shrestha left a comment •
edited
Loading

Choose a reason for hiding this comment

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

why is your PR missing information about #10675. So confusing. I think you need to update bug report title to be more specific and this PR title too.

@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch from f07448a to e1c1d14 Compare January 15, 2025 09:21
@nirajacharya2 nirajacharya2 changed the title [tests-only][full-ci] adding test for deleting auth-app token [tests-only][full-ci] adding test for deleting auth-app token using the token received from auth-app list request Jan 16, 2025
@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch from e1c1d14 to aa37bda Compare January 16, 2025 04:00
@nirajacharya2
Copy link
Contributor Author

IMO its better if other edge case added too in this PR

#10921

@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch 3 times, most recently from ec0a773 to 4b3aa45 Compare January 23, 2025 10:42
saw-jan
saw-jan previously approved these changes Jan 24, 2025
View reviewed changes
@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch 4 times, most recently from b78e67a to d76a50c Compare January 27, 2025 06:04
@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch 2 times, most recently from 7fca83d to 3e7056a Compare February 3, 2025 03:50
@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch from 3e7056a to f063273 Compare February 3, 2025 10:16
@nirajacharya2 nirajacharya2 requested a review from saw-jan February 3, 2025 10:55
amrita-shrestha
amrita-shrestha reviewed Feb 4, 2025
View reviewed changes
tests/acceptance/features/cliCommands/authAppToken.feature
Then the HTTP status code should be "200"
And user "Admin" should have "0" auth-app tokens
When user "Admin" lists all created tokens using the auth-app API
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we have cli command to list tokens

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, i don't see list command in the docs. https://owncloud.dev/services/auth-app/#via-cli-dev-only
and not in the cli tool too ocis/bin/ocis auth-app --help

NAME:
   ocis auth-app - auth-app service commands

USAGE:
   ocis auth-app command [command options]

COMMANDS:
   help, h  Shows a list of commands or help for one command
   info:
     health   check health status
     version  print the version of this binary and the running service instances
   maintenance:
     create  create an app auth token for a user
   server:
     server  start the auth-app service without runtime (unsupervised mode)

OPTIONS:
   --help, -h  show help

tests/acceptance/features/apiAuthApp/token.feature Outdated Show resolved Hide resolved
amrita-shrestha
amrita-shrestha requested changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@amrita-shrestha amrita-shrestha left a comment

Choose a reason for hiding this comment

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

Did you checked edge cases like

  • admin creating app-token for alice user and alice user deleting that toke
  • alice user creating token for him/her self and admin deleting that token
  • admin creating token for alice, brian tries to delete alice token

@saw-jan
Copy link
Member

saw-jan commented Feb 4, 2025 •
edited by amrita-shrestha
Loading

Did you checked edge cases like

  • admin creating app-token for alice user and alice user deleting that toke
  • alice user creating token for him/her self and admin deleting that token
  • admin creating token for alice, brian tries to delete alice token

@nirajacharya2 Please, add them to the QA ticket

@nirajacharya2
Copy link
Contributor Author

Did you checked edge cases like

* admin creating app-token for alice user and alice user deleting that toke

it is covered here
https://github.com/owncloud/ocis/pull/10814/files#diff-4523ee795e0f4bedc559f73001d83c77cb28a9f59ccbfc0b6fd6d2b208b395bcR118-R119

* alice user creating token for him/her self and admin deleting that token

#10921

* admin creating token for alice, brian tries to delete alice token

#10921

@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch from f063273 to 559b7cf Compare February 5, 2025 06:54
@nirajacharya2
Copy link
Contributor Author

@nirajacharya2 Please, add them to the QA ticket

it was mentioned in the issue as try to delete token by a different user under Impersonation. ive edited the point to be more clear.

#10619

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Feb 5, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@nirajacharya2 nirajacharya2 force-pushed the add-auth-app-delete-token-test branch from 559b7cf to a8230b1 Compare February 6, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amrita-shrestha amrita-shrestha amrita-shrestha approved these changes

@phil-davis phil-davis Awaiting requested review from phil-davis

@SagarGi SagarGi Awaiting requested review from SagarGi

@prashant-gurung899 prashant-gurung899 Awaiting requested review from prashant-gurung899

@PrajwolAmatya PrajwolAmatya Awaiting requested review from PrajwolAmatya

@nabim777 nabim777 Awaiting requested review from nabim777

@saw-jan saw-jan Awaiting requested review from saw-jan

Assignees

@nirajacharya2 nirajacharya2

Labels
QA:team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nirajacharya2 @nabim777 @saw-jan @prashant-gurung899 @amrita-shrestha @PrajwolAmatya