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

Fix Service Header to use upper case value #1613

Merged
merged 5 commits into from
Jun 26, 2024

Conversation

Madhu1029
Copy link
Contributor

Fixes #1528. Now, uppercase value can also be used for service header in case of update and delete.

Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM

@fgalan
Copy link
Member

fgalan commented Jun 6, 2024

Note the merge conflict. It should be easy to solve, just adding the existing and new lines.

@mapedraza
Copy link
Collaborator

The PR looks good, thank you so much for your contribution. Being said that, I have a couple of comments:

  1. it would be great if you could add a test to prove it works propperly. It should prove that creating a group having the FIWARE-service header with capitals can be deleted also using the same FIWARE-service value
  2. This fix is valid for group update and delete as checkServiceIdentity() function is only used for those 2 methods. What about the rest of the requests? (e.g: group read and list and device CRUDL)

@Madhu1029
Copy link
Contributor Author

Madhu1029 commented Jun 13, 2024

Hi @mapedraza ,

The PR looks good, thank you so much for your contribution. Being said that, I have a couple of comments:

  1. it would be great if you could add a test to prove it works propperly. It should prove that creating a group having the FIWARE-service header with capitals can be deleted also using the same FIWARE-service value.

I have added test case for deletion and updation with FIWARE-service in uppercase.

  1. This fix is valid for group update and delete as checkServiceIdentity() function is only used for those 2 methods. What about the rest of the requests? (e.g: group read and list and device CRUDL)

As mentioned in issue, the bug exists for deletion and updation only. Group create and read is working fine.

@Madhu1029
Copy link
Contributor Author

Hi @fgalan ,

If the PR seems OK, please merge the PR.

@fgalan fgalan merged commit 093c96f into telefonicaid:master Jun 26, 2024
8 checks passed
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.

Service header inconsistently case sensitive
3 participants