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

🐘 Add like-service (PHP) for liking/unliking posts #63

Merged
merged 11 commits into from
Nov 20, 2023

Conversation

eliasgierlinger
Copy link
Contributor

The new like-service uses Laravel, a PHP framework, in a version that is vulnerable to a certain kind of SQL Injection (see Snyk entry) and that is recognized as a Third Party Vulnerability.

Posts on the frontend now have a like symbol in the bottom right corner, which you can use to like or unlike the post. The Laravel vulnerability allows you to remove another user's like (described in more detail in exploit-toolkit/exploits/sql-injection/SQLI-LIKE-SERVICE-REMOVE-LIKE.md).

The microblog-service had to be updated to send a post ID when you request posts in order to allow the frontend and like-service to uniquely identify posts.

The user-simulator, malicious-load-generator, and exploit-toolkit were also updated.

Some updates had to be made to the way Jaeger is used. Whereas most services communicate with Jaeger via a jaeger-agent, the PHP OpenTelemetry extension can only communicate with the jaeger-collector. However, to get the jaeger-collector to accept OpenTelemetry traces, a config file (chart/jaeger-otlp-values.yaml) needs to be passed when installing jaeger, as described in docs/TRACING.md.

Most of the new files in src/like-service were auto-generated as part of a template. Some of the main files of interest are:

  • routes/web.php: Route definitions
  • app/Http/Controllers/LikeController.php: Handling of the routes
  • config/database.php: Database credentials
  • app/Console/Commands/CreateDatabase.php: Database creation
  • database/migrations/2023_07_13_073935_create_like_table.php: Table creation (yes, the name has to contain a timestamp)
  • public/index.php: Initialization code for tracing (OpenTelemetry/Jaeger)
  • app/Http/Controllers/JaegerPropagator.php: Special "propagator" code that is needed for PHP OpenTelemetry to interface with Jaeger

@eliasgierlinger eliasgierlinger requested a review from a team as a code owner October 11, 2023 11:08
@dynatrace-cla-bot
Copy link
Member

dynatrace-cla-bot commented Oct 11, 2023

CLA assistant check
All committers have signed the CLA.

@W3D3
Copy link
Contributor

W3D3 commented Oct 24, 2023

[ISSUE] The like counter has a serif font, which is not used anywhere else, please change it to the same font that is used everywhere else. Also the whole like element is way too big IMO.
image
Also make sure the paddings are equal on all sides (not like in the screenshot)

Copy link
Contributor

@W3D3 W3D3 left a comment

Choose a reason for hiding this comment

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

Didn't have a look at everything yet, but I left some feedback already that is actionable.

src/frontend/static/img/thumb_up.png Outdated Show resolved Hide resolved
src/frontend/views/base.njk Outdated Show resolved Hide resolved
src/frontend/views/post.njk Outdated Show resolved Hide resolved
src/frontend/views/post.njk Outdated Show resolved Hide resolved
src/like-service/Dockerfile Outdated Show resolved Hide resolved
src/like-service/phpunit.xml Outdated Show resolved Hide resolved
src/frontend/site.js Outdated Show resolved Hide resolved
src/frontend/views/post.njk Outdated Show resolved Hide resolved
src/frontend/views/post.njk Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/envoy-proxy/config/envoy-config.yaml Outdated Show resolved Hide resolved
chart/templates/ingress.yaml Outdated Show resolved Hide resolved
src/frontend/site.js Outdated Show resolved Hide resolved
chart/jaeger-otlp-values.yaml Outdated Show resolved Hide resolved
docs/TRACING.md Show resolved Hide resolved
exploit-toolkit/exploits/sql-injection/README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
chart/templates/ingress.yaml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
src/like-service/README.md Outdated Show resolved Hide resolved
src/like-service/README.md Outdated Show resolved Hide resolved
src/frontend/site.js Outdated Show resolved Hide resolved
Copy link
Contributor

@W3D3 W3D3 left a comment

Choose a reason for hiding this comment

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

Tried it out locally again, works like a charm, including the exploit toolkit 👍
LGTM!

@W3D3
Copy link
Contributor

W3D3 commented Nov 20, 2023

Pls rebase on main and then we'll merge this :)

@eliasgierlinger
Copy link
Contributor Author

@W3D3 I rebased; you should be able to merge it now 🚀

@W3D3 W3D3 merged commit b738608 into dynatrace-oss:main Nov 20, 2023
4 checks passed
@eliasgierlinger eliasgierlinger deleted the main_php branch November 21, 2023 07:10
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.

5 participants